Updated page content related links on content id changes

For #2278

diff --git a/app/Entities/Managers/PageContent.php b/app/Entities/Managers/PageContent.php
index e417b1caad6e2b07bb457889c3b1489354e942ad..a787e5d993a0d31b29519ccbc29871881b918d9e 100644 (file)
--- a/app/Entities/Managers/PageContent.php
+++ b/app/Entities/Managers/PageContent.php
@@ -2,7 +2,6 @@
 
 use BookStack\Entities\Page;
 use DOMDocument;
 
 use BookStack\Entities\Page;
 use DOMDocument;

-use DOMElement;

 use DOMNodeList;
 use DOMXPath;
 
 use DOMNodeList;
 use DOMXPath;
 


@@ -44,18 +43,24 @@ class PageContent
         $container = $doc->documentElement;
         $body = $container->childNodes->item(0);
         $childNodes = $body->childNodes;
         $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) {
 
         // 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
         }
 
         // Ensure no duplicate ids within child items

-        $xPath = new DOMXPath($doc);

         $idElems = $xPath->query('//body//*//*[@id]');
         foreach ($idElems as $domElem) {
         $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
         }
 
         // Generate inner html as a string


@@ -67,23 +72,34 @@ class PageContent
         return $html;
     }
 
         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.
     /**
      * 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') {
     {
         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;
         $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
         }
 
         // Create an unique id for the element


@@ -100,6 +116,7 @@ class PageContent
 
         $element->setAttribute('id', $newId);
         $idMap[$newId] = true;
 
         $element->setAttribute('id', $newId);
         $idMap[$newId] = true;

+        return [$existingId, $newId];

     }
 
     /**
     }
 
     /**

diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php
index 69b46b06e4e73b43bbfa290dd93faa9716977107..99547fd1798563d414cc5aa3355a93eee923868d 100644 (file)
--- 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);
     }
 
         $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 = '<h1 id="non-standard-id">test</h1><p><a href="#non-standard-id">link</a></p>';
+        $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 = '<h1 id="testa">Hello</h1><h2 id="testb">There</h2><h3 id="testc">Donkey</h3>';
     public function test_get_page_nav_sets_correct_properties()
     {
         $content = '<h1 id="testa">Hello</h1><h2 id="testb">There</h2><h3 id="testc">Donkey</h3>';