From 465d405926c7e22173fe650bf2d417d4633d7990 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 28 Sep 2020 22:26:50 +0100 Subject: [PATCH] Updated page content related links on content id changes For #2278 --- app/Entities/Managers/PageContent.php | 37 +++++++++++++++++++-------- tests/Entity/PageContentTest.php | 17 ++++++++++++ 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/app/Entities/Managers/PageContent.php b/app/Entities/Managers/PageContent.php index e417b1caa..a787e5d99 100644 --- a/app/Entities/Managers/PageContent.php +++ b/app/Entities/Managers/PageContent.php @@ -2,7 +2,6 @@ use BookStack\Entities\Page; use DOMDocument; -use DOMElement; use DOMNodeList; use DOMXPath; @@ -44,18 +43,24 @@ class PageContent $container = $doc->documentElement; $body = $container->childNodes->item(0); $childNodes = $body->childNodes; + $xPath = new DOMXPath($doc); // Set ids on top-level nodes $idMap = []; foreach ($childNodes as $index => $childNode) { - $this->setUniqueId($childNode, $idMap); + [$oldId, $newId] = $this->setUniqueId($childNode, $idMap); + if ($newId && $newId !== $oldId) { + $this->updateLinks($xPath, '#' . $oldId, '#' . $newId); + } } // Ensure no duplicate ids within child items - $xPath = new DOMXPath($doc); $idElems = $xPath->query('//body//*//*[@id]'); foreach ($idElems as $domElem) { - $this->setUniqueId($domElem, $idMap); + [$oldId, $newId] = $this->setUniqueId($domElem, $idMap); + if ($newId && $newId !== $oldId) { + $this->updateLinks($xPath, '#' . $oldId, '#' . $newId); + } } // Generate inner html as a string @@ -67,23 +72,34 @@ class PageContent return $html; } + /** + * Update the all links to the $old location to instead point to $new. + */ + protected function updateLinks(DOMXPath $xpath, string $old, string $new) + { + $old = str_replace('"', '', $old); + $matchingLinks = $xpath->query('//body//*//*[@href="'.$old.'"]'); + foreach ($matchingLinks as $domElem) { + $domElem->setAttribute('href', $new); + } + } + /** * Set a unique id on the given DOMElement. * A map for existing ID's should be passed in to check for current existence. - * @param DOMElement $element - * @param array $idMap + * Returns a pair of strings in the format [old_id, new_id] */ - protected function setUniqueId($element, array &$idMap) + protected function setUniqueId(\DOMNode $element, array &$idMap): array { if (get_class($element) !== 'DOMElement') { - return; + return ['', '']; } - // Overwrite id if not a BookStack custom id + // Stop if there's an existing valid id that has not already been used. $existingId = $element->getAttribute('id'); if (strpos($existingId, 'bkmrk') === 0 && !isset($idMap[$existingId])) { $idMap[$existingId] = true; - return; + return [$existingId, $existingId]; } // Create an unique id for the element @@ -100,6 +116,7 @@ class PageContent $element->setAttribute('id', $newId); $idMap[$newId] = true; + return [$existingId, $newId]; } /** diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php index 69b46b06e..99547fd17 100644 --- a/tests/Entity/PageContentTest.php +++ b/tests/Entity/PageContentTest.php @@ -262,6 +262,23 @@ class PageContentTest extends TestCase $this->assertEquals(substr_count($updatedPage->html, "bkmrk-test\""), 1); } + public function test_anchors_referencing_non_bkmrk_ids_rewritten_after_save() + { + $this->asEditor(); + $page = Page::first(); + + $content = '

test

link

'; + $this->put($page->getUrl(), [ + 'name' => $page->name, + 'html' => $content, + 'summary' => '' + ]); + + $updatedPage = Page::where('id', '=', $page->id)->first(); + $this->assertStringContainsString('id="bkmrk-test"', $updatedPage->html); + $this->assertStringContainsString('href="#bkmrk-test"', $updatedPage->html); + } + public function test_get_page_nav_sets_correct_properties() { $content = '

Hello

There

Donkey

';