Fix section ID deduplication by looping through the whole node tree

[wouterj]

The compiler pass resolving conflicts in section IDs only looped one level deep through the nodes. This means only the "main" document section was checked for conflicts, instead of all subsections of the document.

---

There is another problem with section IDs that I'm investigating, which is related to the MoveAnchorTransformer when using explicit hyperlink targets. The AnchorNode is copied into the section node by this transformer, but not removed from the original location. Leading to duplicate anchors like:

        ...
        <a id="something-different"></a>
    </div>
    <div class="section" id="another-subtitle">
        <a id="something-different"></a>
        <h2>
            Another Subtitle
        </h2>
        ...

But as that looks a bit more complex, I'll open a separate PR once I find the fix for this issue.

[wouterj]

This code is redundant: Explicit anchors don't override implicit section IDs. They add additional section IDs.

    <div class="section" id="another-subtitle">
        <a id="something-different"></a>

The menu logic can keep using the implicit references, reducing complexity in this compiler pass.

[jaapio]

Thanks, for this improvement. Really nice to see you are making progress.

[jaapio]

Regarding nodes that are not removed. Is this at the root level of documents? I remember that I found something was wrong there.
I need to check if I fixed that already.

[wouterj]

Regarding nodes that are not removed. Is this at the root level of documents? I remember that I found something was wrong there.

If you add this patch to the test fixture introduced in this PR, you can observe it:

diff --git a/tests/Functional/tests/titles-ids/titles-ids.html b/tests/Functional/tests/titles-ids/titles-ids.html
index 78d4e577..e98e5038 100644
--- a/tests/Functional/tests/titles-ids/titles-ids.html
+++ b/tests/Functional/tests/titles-ids/titles-ids.html
@@ -24,4 +24,18 @@
         </h2>
         <p>This also works for titles with markup.</p>
     </div>
+    <div class="section" id="another-subtitle">
+        <a id="something-different"></a>
+        <h2>
+            Another Subtitle
+        </h2>
+        <p>You can override subtitle IDs by placing an anchor before the section.</p>
+        <div class="section" id="subsubtitle">
+            <a id="something"></a>
+            <h3>
+                Subsubtitle
+            </h3>
+            <p>This works on all levels.</p>
+        </div>
+    </div>
 </div>
diff --git a/tests/Functional/tests/titles-ids/titles-ids.rst b/tests/Functional/tests/titles-ids/titles-ids.rst
index 33f2c7ca..244c8bf5 100644
--- a/tests/Functional/tests/titles-ids/titles-ids.rst
+++ b/tests/Functional/tests/titles-ids/titles-ids.rst
@@ -16,3 +16,17 @@ Other ``Subtitle``
 ------------------
 
 This also works for titles with markup.
+
+.. _something-different:
+
+Another Subtitle
+----------------
+
+You can override subtitle IDs by placing an anchor before the section.
+
+.. _something:
+
+Subsubtitle
+~~~~~~~~~~~
+
+This works on all levels.

This makes the test fail with:

--- Expected
+++ Actual
@@ @@
             Other <code>Subtitle</code>
         </h2>
         <p>This also works for titles with markup.</p>
+        <a id="something-different"></a>
     </div>
     <div class="section" id="another-subtitle">
         <a id="something-different"></a>
@@ @@
             Another Subtitle
         </h2>
         <p>You can override subtitle IDs by placing an anchor before the section.</p>
+        <a id="something"></a>
         <div class="section" id="subsubtitle">
             <a id="something"></a>
             <h3>

But I somehow can't reproduce this in the Symfony docs-builder test suite (where we test explicit anchors as well). I've traced it back to

guides/packages/guides/src/Compiler/DocumentNodeTraverser.php

Line 80 in be8d211

$shadowNode->getParent()?->removeChild($node);

Somehow TreeNode::removeNode() doesn't appear to remove the node in the testsuite.