Issue #3360540 by RoSk0, ericgsmith: Duplicated HTML IDs

src/Toc.php

@@ -104,6 +104,13 @@ class Toc implements TocInterface {
    */
   protected $tree;
 
+  /**
+   * Existing in the source content IDs.
+   *
+   * @var array
+   */
+  protected $existingIds = [];
+
   /**
    * Constructs a new TOC object.
    *
@@ -206,22 +213,29 @@ class Toc implements TocInterface {
     $index_keys = $default_keys;
 
     $dom = Html::load($this->source);
+    $xpath = new \DOMXPath($dom);
 
     // If the exclude XPath option is set, retrieve those elements.
     $exclude_nodes = [];
     if ($this->options['header_exclude_xpath']) {
-      $xpath = new \DOMXPath($dom);
       foreach ($xpath->query($this->options['header_exclude_xpath']) as $exclude_node) {
         $exclude_nodes[] = $exclude_node;
       }
     }
 
-    // Loop through all the tags to ensure headers are found in the correct
-    // order.
-    $dom_nodes = $dom->getElementsByTagName('*');
-    /** @var \DOMElement $dom_node */
-    foreach ($dom_nodes as $dom_node) {
-      if (empty($this->options['headers'][$dom_node->tagName]) || in_array($dom_node, $exclude_nodes, TRUE)) {
+    $this->useIdsOnly($dom);
+    $this->collectExistingIds($dom);
+
+    // Loop through all configured headers.
+    $predicates = array_map(function ($header) {
+      return 'self::' . $header;
+    }, array_keys($this->options['headers']));
+    $query = '//*[' . implode(' or ', $predicates) . ']';
+    // According to specification, https://www.w3.org/TR/xpath-31/#id-steps,
+    // "The resulting node sequence is returned in document order."
+    foreach ($xpath->query($query) as $dom_node) {
+      /** @var \DOMElement $dom_node */
+      if (in_array($dom_node, $exclude_nodes, TRUE)) {
         continue;
       }
 
@@ -332,6 +346,7 @@ class Toc implements TocInterface {
         ),
       ];
     }
+
     $this->content = Html::serialize($dom);
   }
 
@@ -471,7 +486,7 @@ class Toc implements TocInterface {
   protected function uniqueId($id) {
     $unique_id = $id;
     $i = 1;
-    while (isset($this->ids[$id])) {
+    while (isset($this->ids[$id]) || isset($this->existingIds[$id])) {
       $unique_suffix = '-' . sprintf("%02s", $i);
       $id = $unique_id . $unique_suffix;
       $i++;
@@ -479,4 +494,45 @@ class Toc implements TocInterface {
     return $id;
   }
 
+  /**
+   * Collect existing IDs.
+   *
+   * @param \DOMDocument $dom
+   *   DOM object.
+   *
+   * @return void
+   */
+  protected function collectExistingIds(\DOMDocument $dom): void {
+    $xpath = new \DOMXPath($dom);
+    // All elements with 'id' attribute set.
+    $query = '//*[@id]';
+    foreach ($xpath->query($query) as $dom_node) {
+      /** @var \DOMElement $dom_node */
+      if ($dom_node->getAttribute('id')) {
+        $this->existingIds[$dom_node->getAttribute('id')] = $dom_node->getAttribute('id');
+      }
+    }
+  }
+
+  /**
+   * Re-assigns the value of deprecated "name" attribute to "id" attribute.
+   *
+   * @param \DOMDocument $dom
+   *   DOM object.
+   * @return void
+   */
+  protected function useIdsOnly(\DOMDocument $dom): void {
+    $xpath = new \DOMXPath($dom);
+    // All elements with 'name' attribute set and 'id' attribute missing or
+    // empty.
+    $query = '//*[@name and (not(@id) or string-length(@id)=0)]';
+    foreach ($xpath->query($query) as $dom_node) {
+      /** @var \DOMElement $dom_node */
+      if ($dom_node->getAttribute('name') && empty($dom_node->getAttribute('id'))) {
+        $dom_node->setAttribute('id', $dom_node->getAttribute('name'));
+        $dom_node->removeAttribute('name');
+      }
+    }
+  }
+
 }

tests/src/Kernel/TocBuilderTest.php

+<?php
+
+namespace Drupal\Tests\toc_api\Kernel;
+
+use Drupal\Component\Utility\Html;
+use Drupal\KernelTests\KernelTestBase;
+use Drupal\toc_api\TocBuilderInterface;
+use Drupal\toc_api\TocManager;
+use Drupal\toc_api\TocManagerInterface;
+
+/**
+ * Test description.
+ *
+ * @group toc_api
+ */
+class TocBuilderTest extends KernelTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['toc_api'];
+
+  /**
+   * @var \Drupal\toc_api\TocManagerInterface
+   */
+  protected TocManagerInterface $manager;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+
+    $this->installEntitySchema('toc_type');
+    $this->installConfig(['toc_api']);
+    $this->manager = new TocManager($this->container->get('config.factory'));
+  }
+
+  /**
+   * Tests that ToC doesn't produce duplicated IDs.
+   *
+   * @todo Convert to unit test and include in TocTest, once that is fixed.
+   */
+  public function testDuplicateIds() {
+    $toc_source = <<<HTML
+    <p><a name="documents"></a></p>
+    <h2>Documents</h2>
+    <p>
+      Tests that ToC API doesn't get confused by the deprecated "name" attribute.
+      It was present in the XHTML 1.0, see https://www.w3.org/TR/xhtml1/#h-4.10 ,
+      but removed from XHTML 1.1 , see the list of included modules for it
+      here https://www.w3.org/TR/xhtml11/doctype.html , and deprecated
+      "Name Identification Module" description here
+      https://www.w3.org/TR/xhtml-modularization/abstract_modules.html .
+    </p>
+
+    <p><a id="documents-heading-3"></a></p>
+    <h3>Documents heading 3</h3>
+
+    <h4>Documents heading 4</h4>
+    <p>It is important that existing ID comes after the heading here.</p>
+    <p><a id="documents-heading-4"></a></p>
+    HTML;
+    $toc_options = [
+      'header_min' => '2',
+      'header_max' => '4',
+      'header_id' => 'title',
+    ];
+
+    $toc = $this->manager->create('toc_manager_test', $toc_source, $toc_options);
+    $dom = Html::load($toc->getContent());
+
+    // Test that ToC API doesn't get confused by existing deprecated "name"
+    // attribute.
+    $h2 = $dom->getElementsByTagName('h2')->item(0);
+    $this->assertNotEquals('documents' , $h2->getAttribute('id'), 'H2 have unique ID');
+
+    $h3 = $dom->getElementsByTagName('h3')->item(0);
+    $this->assertNotEquals('documents-heading-3' , $h3->getAttribute('id'), 'H3 have unique ID');
+
+    $h4 = $dom->getElementsByTagName('h4')->item(0);
+    $this->assertNotEquals('documents-heading-4' , $h4->getAttribute('id'), 'H4 have unique ID');
+  }
+
+}