Merge pull request #1149 from Patternslib/fix--scroll-marker--unescaped

Fix  scroll marker  unescaped

Author: thet

package.json

@@ -44,6 +44,7 @@
         "@patternslib/pat-tiptap": "^4.8.2",
         "@patternslib/pat-upload": "^3.1.1",
         "copy-webpack-plugin": "^11.0.0",
+        "css.escape": "^1.5.1",
         "modernizr": "^3.12.0",
         "pegjs": "0.11.0-master.b7b87ea"
     },

src/core/dom.js

@@ -410,6 +410,29 @@ const get_visible_ratio = (el, container) => {
     return visible_ratio;
 };
 
+/**
+ * Get an escaped CSS selector for a given id string.
+ *
+ * id selectors should - but don't have to - start with a letter.
+ * If the id starts with a number or a dash, it should be escaped.
+ * This method does that for you.
+ *
+ * Alse see:
+ * - https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id
+ * - https://developer.mozilla.org/en-US/docs/Web/API/CSS/escape
+ *
+ * @param {String} id - The id to escape.
+ *
+ * @returns {String} - The escaped CSS selector.
+ *
+ * @example
+ * escape_css_id_selector("#123"); // returns "#\\31 23""
+ * escape_css_id_selector("#-123"); // returns "#-\\31 23"
+ */
+const escape_css_id = (id) => {
+    return `#${CSS.escape(id.split("#")[1])}`;
+};
+
 const dom = {
     toNodeArray: toNodeArray,
     querySelectorAllAndMe: querySelectorAllAndMe,
@@ -432,6 +455,7 @@ const dom = {
     delete_data: delete_data,
     template: template,
     get_visible_ratio: get_visible_ratio,
+    escape_css_id: escape_css_id,
     add_event_listener: events.add_event_listener, // BBB export. TODO: Remove in an upcoming version.
     remove_event_listener: events.remove_event_listener, // BBB export. TODO: Remove in an upcoming version.
 };

src/core/dom.test.js

@@ -856,3 +856,21 @@ describe("core.dom tests", () => {
         });
     });
 });
+
+describe("escape_css_id", function () {
+    it("returns a standard id selector as-is", function () {
+        expect(dom.escape_css_id("#foo")).toBe("#foo");
+    });
+
+    it("returns an escaped version when the id is a number", function () {
+        expect(dom.escape_css_id("#123")).toBe("#\\31 23");
+    });
+
+    it("returns an escaped version when the id starts with a number", function () {
+        expect(dom.escape_css_id("#1foo")).toBe("#\\31 foo");
+    });
+
+    it("returns an escaped version when the id starts with a dash", function () {
+        expect(dom.escape_css_id("#-1-2-3")).toBe("#-\\31 -2-3");
+    });
+});

src/pat/navigation/index.html

@@ -113,6 +113,12 @@ <h3>Example 3 - nested nav, but no injection</h3>
                     <a href="#2">Tab 2</a>
                 </li>
             </ul>
+
+            <section id="10">Content for tab 1</section>
+            <section id="11">Content for tab 1.1</section>
+            <section id="12">Content for tab 1.2</section>
+            <section id="2">Content for tab 2</section>
+
         </section>
 
         <template id="inject-tab-1">Content for tab 1</template>

src/pat/navigation/navigation.js

@@ -2,6 +2,7 @@ import $ from "jquery";
 import { BasePattern } from "../../core/basepattern";
 import Parser from "../../core/parser";
 import ScrollMarker from "../scroll-marker/scroll-marker";
+import dom from "../../core/dom";
 import logging from "../../core/logging";
 import events from "../../core/events";
 import registry from "../../core/registry";
@@ -180,7 +181,9 @@ class Pattern extends BasePattern {
             }
             // Mark the current content item, if it is a hash link.
             if (item.matches("a[href^='#']")) {
-                const content_item = document.querySelector(item.getAttribute("href"));
+                const content_item = document.querySelector(
+                    dom.escape_css_id(item.hash)
+                );
                 if (content_item) {
                     content_item.classList.add(this.options["current-content-class"]);
                 }

src/pat/scroll-marker/scroll-marker.js

@@ -31,7 +31,7 @@ class Pattern extends BasePattern {
     // differently, e.g. by clicking in a pat-navigation menu.
     set_current_disabled = false;
 
-    async init() {
+    init() {
         // Get all elements that are referenced by links in the current page.
         this.observables = new Map(
             [...dom.querySelectorAllAndMe(this.el, "a[href^='#']")]
@@ -41,7 +41,10 @@ class Pattern extends BasePattern {
                     // to create the resulting Map holding all necessary information.
                     (it) => [
                         it.hash.split("#")[1],
-                        { link: it, target: document.querySelector(it.hash) },
+                        {
+                            link: it,
+                            target: document.querySelector(dom.escape_css_id(it.hash)),
+                        },
                     ]
                 )
                 .filter(

src/setup-tests.js

@@ -19,3 +19,6 @@ import dom from "./core/dom";
 dom.is_visible = (el) => {
     return !el.hidden && el.style.display !== "none";
 };
+
+// polyfill css.escape for jsdom
+import("css.escape");

yarn.lock

@@ -3774,6 +3774,11 @@ css-loader@^6.7.3:
     postcss-value-parser "^4.2.0"
     semver "^7.3.8"
 
+css.escape@^1.5.1:
+  version "1.5.1"
+  resolved "https://registry.yarnpkg.com/css.escape/-/css.escape-1.5.1.tgz#42e27d4fa04ae32f931a4b4d4191fa9cddee97cb"
+  integrity sha512-YUifsXXuknHlUsmlgyY0PKzgPOr7/FjCePfHNt0jxm83wHZi44VDMQ7/fGNkjY3/jV1MC+1CmZbaHzugyeRtpg==
+
 cssesc@^3.0.0:
   version "3.0.0"
   resolved "https://registry.yarnpkg.com/cssesc/-/cssesc-3.0.0.tgz#37741919903b868565e1c09ea747445cd18983ee"