Title: [266815] trunk
Revision
266815
Author
commit-qu...@webkit.org
Date
2020-09-09 23:55:45 -0700 (Wed, 09 Sep 2020)

Log Message

Web Inspector: InvalidCharacterError: The string contains invalid characters.
https://bugs.webkit.org/show_bug.cgi?id=216138

Patch by Patrick Angle <pan...@apple.com> on 2020-09-09
Reviewed by Brian Burg.

Source/WebInspectorUI:

Fixed to no longer use displayName as the identifier for `DOMNodeDetailsSidebarPanel` when creating a
`DetailsSection`, which could end up with illegal characters due to the escaping done by
`DOMNode.prototype.displayName`.

* UserInterface/Models/DOMNode.js:
(WI.DOMNode.prototype.get escapedIdSelector): Changed to use `_idSelector`.
(WI.DOMNode.prototype.get escapedClassSelector): Changed to use `_classSelector`.
(WI.DOMNode.prototype.get unescapedSelector): Added. Uses unescaped forms of `_idSelector` and `_classSelector`.
(WI.DOMNode.prototype._idSelector): Added. Supports optionally not escaping the selector.
(WI.DOMNode.prototype._classSelector): Added. Supports optionally not escaping the selector.
* UserInterface/Views/DOMNodeDetailsSidebarPanel.js:
(WI.DOMNodeDetailsSidebarPanel.prototype._refreshEventListeners.generateGroupsByTarget): Use separate identifier
and title.

LayoutTests:

Added tests for `DOMNode.prototype._idSelector(…)`, `DOMNode.prototype._classSelector(…)`,
`DOMNode.prototype.unescapedSelector` and `DOMNode.prototype.displayName`.

* inspector/dom/selector-escapes-expected.txt: Added.
* inspector/dom/selector-escapes.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (266814 => 266815)


--- trunk/LayoutTests/ChangeLog	2020-09-10 05:01:13 UTC (rev 266814)
+++ trunk/LayoutTests/ChangeLog	2020-09-10 06:55:45 UTC (rev 266815)
@@ -1,3 +1,16 @@
+2020-09-09  Patrick Angle  <pan...@apple.com>
+
+        Web Inspector: InvalidCharacterError: The string contains invalid characters.
+        https://bugs.webkit.org/show_bug.cgi?id=216138
+
+        Reviewed by Brian Burg.
+
+        Added tests for `DOMNode.prototype._idSelector(…)`, `DOMNode.prototype._classSelector(…)`,
+        `DOMNode.prototype.unescapedSelector` and `DOMNode.prototype.displayName`.
+
+        * inspector/dom/selector-escapes-expected.txt: Added.
+        * inspector/dom/selector-escapes.html: Added.
+
 2020-09-09  Hector Lopez  <hector_i_lo...@apple.com>
 
         [ macOS iOS ] compositing/clipping/border-radius-async-overflow-stacking.html is flaky failing.

Added: trunk/LayoutTests/inspector/dom/selector-escapes-expected.txt (0 => 266815)


--- trunk/LayoutTests/inspector/dom/selector-escapes-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/inspector/dom/selector-escapes-expected.txt	2020-09-10 06:55:45 UTC (rev 266815)
@@ -0,0 +1,28 @@
+Test for DOMNode.SelectorEscapes.
+
+
+== Running test suite: DOMNode.SelectorEscapes
+-- Running test case: DOMNode.SelectorEscapes.NormalId
+PASS: Unescaped id selector should be `#id`.
+PASS: Unescaped class selector should be `.class`.
+PASS: Unescaped selector should be `div#id.class`.
+PASS: Escaped id selector should be `#id`.
+PASS: Escaped class selector should be `.class`.
+PASS: Display name should be `div#id.class`.
+
+-- Running test case: DOMNode.SelectorEscapes.NumberId
+PASS: Unescaped id selector should be `#123Id`.
+PASS: Unescaped class selector should be `.123Class`.
+PASS: Unescaped selector should be `div#123Id.123Class`.
+PASS: Escaped id selector should be `[id="\31 23Id"]`.
+PASS: Escaped class selector should be `.\31 23Class`.
+PASS: Display name should be `div[id="\31 23Id"].\31 23Class`.
+
+-- Running test case: DOMNode.SelectorEscapes.PoundId
+PASS: Unescaped id selector should be `##id`.
+PASS: Unescaped class selector should be `.#class`.
+PASS: Unescaped selector should be `div##id.#class`.
+PASS: Escaped id selector should be `#\#id`.
+PASS: Escaped class selector should be `.\#class`.
+PASS: Display name should be `div#\#id.\#class`.
+

Added: trunk/LayoutTests/inspector/dom/selector-escapes.html (0 => 266815)


--- trunk/LayoutTests/inspector/dom/selector-escapes.html	                        (rev 0)
+++ trunk/LayoutTests/inspector/dom/selector-escapes.html	2020-09-10 06:55:45 UTC (rev 266815)
@@ -0,0 +1,87 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script>
+function test()
+{
+    let documentNode;
+
+    let suite = InspectorTest.createAsyncSuite("DOMNode.SelectorEscapes");
+
+    suite.addTestCase({
+        name: "DOMNode.SelectorEscapes.NormalId",
+        test(resolve, reject) {
+            documentNode.querySelector("#id", (nodeId) => {
+                let domNode = WI.domManager.nodeForId(nodeId);
+                InspectorTest.assert(domNode, "Got DOMNode for id `id`");
+                
+                InspectorTest.expectEqual(domNode._idSelector(false), "#id", "Unescaped id selector should be `#id`.");
+                InspectorTest.expectEqual(domNode._classSelector(false), ".class", "Unescaped class selector should be `.class`.");
+                InspectorTest.expectEqual(domNode.unescapedSelector, "div#id.class", "Unescaped selector should be `div#id.class`.");
+
+                InspectorTest.expectEqual(domNode._idSelector(true), "#id", "Escaped id selector should be `#id`.");
+                InspectorTest.expectEqual(domNode._classSelector(true), ".class", "Escaped class selector should be `.class`.");
+                InspectorTest.expectEqual(domNode.displayName, "div#id.class", "Display name should be `div#id.class`.");
+                
+                resolve();
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "DOMNode.SelectorEscapes.NumberId",
+        test(resolve, reject) {
+            documentNode.querySelector("#\\31 23Id", (nodeId) => {
+                let domNode = WI.domManager.nodeForId(nodeId);
+                InspectorTest.assert(domNode, "Got DOMNode for id `123Id`");
+
+                InspectorTest.expectEqual(domNode._idSelector(false), "#123Id", "Unescaped id selector should be `#123Id`.");
+                InspectorTest.expectEqual(domNode._classSelector(false), ".123Class", "Unescaped class selector should be `.123Class`.");
+                InspectorTest.expectEqual(domNode.unescapedSelector, "div#123Id.123Class", "Unescaped selector should be `div#123Id.123Class`.");
+
+                InspectorTest.expectEqual(domNode._idSelector(true), "[id=\"\\31 23Id\"]", "Escaped id selector should be `[id=\"\\31 23Id\"]`.");
+                InspectorTest.expectEqual(domNode._classSelector(true), ".\\31 23Class", "Escaped class selector should be `.\\31 23Class`.");
+                InspectorTest.expectEqual(domNode.displayName, "div[id=\"\\31 23Id\"].\\31 23Class", "Display name should be `div[id=\"\\31 23Id\"].\\31 23Class`.");
+
+                resolve();
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "DOMNode.SelectorEscapes.PoundId",
+        test(resolve, reject) {
+            documentNode.querySelector("#\\#id", (nodeId) => {
+                let domNode = WI.domManager.nodeForId(nodeId);
+                InspectorTest.assert(domNode, "Got DOMNode for id `#id`");
+                
+                InspectorTest.expectEqual(domNode._idSelector(false), "##id", "Unescaped id selector should be `##id`.");
+                InspectorTest.expectEqual(domNode._classSelector(false), ".#class", "Unescaped class selector should be `.#class`.");
+                InspectorTest.expectEqual(domNode.unescapedSelector, "div##id.#class", "Unescaped selector should be `div##id.#class`.");
+
+                InspectorTest.expectEqual(domNode._idSelector(true), "#\\#id", "Escaped id selector should be `#\\#id`.");
+                InspectorTest.expectEqual(domNode._classSelector(true), ".\\#class", "Escaped class selector should be `.\\#class`.");
+                InspectorTest.expectEqual(domNode.displayName, "div#\\#id.\\#class", "Display name should be `div#\\#id.\\#class`.");
+
+                resolve();
+            });
+        }
+    });
+
+    WI.domManager.requestDocument((node) => {
+        documentNode = node;
+        suite.runTestCasesAndFinish();
+    });
+}
+</script>
+</head>
+<body _onload_="runTest()">
+<p>Test for DOMNode.SelectorEscapes.</p>
+<div style="display: none">
+    <div id="id" class="class"></div>
+    <div id="123Id" class="123Class"></div>
+    <div id="#id" class="#class"></div>
+</div>
+</body>
+</html>

Modified: trunk/Source/WebInspectorUI/ChangeLog (266814 => 266815)


--- trunk/Source/WebInspectorUI/ChangeLog	2020-09-10 05:01:13 UTC (rev 266814)
+++ trunk/Source/WebInspectorUI/ChangeLog	2020-09-10 06:55:45 UTC (rev 266815)
@@ -1,3 +1,24 @@
+2020-09-09  Patrick Angle  <pan...@apple.com>
+
+        Web Inspector: InvalidCharacterError: The string contains invalid characters.
+        https://bugs.webkit.org/show_bug.cgi?id=216138
+
+        Reviewed by Brian Burg.
+
+        Fixed to no longer use displayName as the identifier for `DOMNodeDetailsSidebarPanel` when creating a
+        `DetailsSection`, which could end up with illegal characters due to the escaping done by
+        `DOMNode.prototype.displayName`.
+
+        * UserInterface/Models/DOMNode.js:
+        (WI.DOMNode.prototype.get escapedIdSelector): Changed to use `_idSelector`.
+        (WI.DOMNode.prototype.get escapedClassSelector): Changed to use `_classSelector`.
+        (WI.DOMNode.prototype.get unescapedSelector): Added. Uses unescaped forms of `_idSelector` and `_classSelector`.
+        (WI.DOMNode.prototype._idSelector): Added. Supports optionally not escaping the selector.
+        (WI.DOMNode.prototype._classSelector): Added. Supports optionally not escaping the selector.
+        * UserInterface/Views/DOMNodeDetailsSidebarPanel.js:
+        (WI.DOMNodeDetailsSidebarPanel.prototype._refreshEventListeners.generateGroupsByTarget): Use separate identifier
+        and title.
+
 2020-09-08  Patrick Angle  <pan...@apple.com>
 
         Web Inspector: Incorrect decimal separator in Network web inspector

Modified: trunk/Source/WebInspectorUI/UserInterface/Models/DOMNode.js (266814 => 266815)


--- trunk/Source/WebInspectorUI/UserInterface/Models/DOMNode.js	2020-09-10 05:01:13 UTC (rev 266814)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/DOMNode.js	2020-09-10 06:55:45 UTC (rev 266815)
@@ -757,39 +757,12 @@
 
     get escapedIdSelector()
     {
-        let id = this.getAttribute("id");
-        if (!id)
-            return "";
-
-        id = id.trim();
-        if (!id.length)
-            return "";
-
-        id = CSS.escape(id);
-        if (/[\s'"]/.test(id))
-            return `[id="${id}"]`;
-
-        return `#${id}`;
+        return this._idSelector(true);
     }
 
     get escapedClassSelector()
     {
-        let classes = this.getAttribute("class");
-        if (!classes)
-            return "";
-
-        classes = classes.trim();
-        if (!classes.length)
-            return "";
-
-        let foundClasses = new Set;
-        return classes.split(/\s+/).reduce((selector, className) => {
-            if (!className.length || foundClasses.has(className))
-                return selector;
-
-            foundClasses.add(className);
-            return `${selector}.${CSS.escape(className)}`;
-        }, "");
+        return this._classSelector(true);
     }
 
     get displayName()
@@ -799,6 +772,15 @@
         return this.nodeNameInCorrectCase() + this.escapedIdSelector + this.escapedClassSelector;
     }
 
+    get unescapedSelector()
+    {
+        if (this.isPseudoElement())
+            return "::" + this._pseudoType;
+
+        const shouldEscape = false;
+        return this.nodeNameInCorrectCase() + this._idSelector(shouldEscape) + this._classSelector(shouldEscape);
+    }
+
     appropriateSelectorFor(justSelector)
     {
         if (this.isPseudoElement())
@@ -1066,6 +1048,43 @@
                 callback.apply(null, args);
         };
     }
+
+    _idSelector(shouldEscape)
+    {
+        let id = this.getAttribute("id");
+        if (!id)
+            return "";
+
+        id = id.trim();
+        if (!id.length)
+            return "";
+
+        if (shouldEscape)
+            id = CSS.escape(id);
+        if (/[\s'"]/.test(id))
+            return `[id="${id}"]`;
+
+        return `#${id}`;
+    }
+
+    _classSelector(shouldEscape) {
+        let classes = this.getAttribute("class");
+        if (!classes)
+            return "";
+
+        classes = classes.trim();
+        if (!classes.length)
+            return "";
+
+        let foundClasses = new Set;
+        return classes.split(/\s+/).reduce((selector, className) => {
+            if (!className.length || foundClasses.has(className))
+                return selector;
+
+            foundClasses.add(className);
+            return `${selector}.${(shouldEscape ? CSS.escape(className) : className)}`;
+        }, "");
+    }
 };
 
 WI.DOMNode.Event = {

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DOMNodeDetailsSidebarPanel.js (266814 => 266815)


--- trunk/Source/WebInspectorUI/UserInterface/Views/DOMNodeDetailsSidebarPanel.js	2020-09-10 05:01:13 UTC (rev 266814)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DOMNodeDetailsSidebarPanel.js	2020-09-10 06:55:45 UTC (rev 266815)
@@ -354,7 +354,8 @@
             }
 
             const defaultCollapsedSettingValue = true;
-            let section = new WI.DetailsSection(`${title}-event-listener-section`, title, groups, optionsElement, defaultCollapsedSettingValue);
+            let identifier = `${options.identifier ?? title}-event-listener-section`
+            let section = new WI.DetailsSection(identifier, title, groups, optionsElement, defaultCollapsedSettingValue);
             section.element.classList.add("event-listener-section");
             return section;
         }
@@ -406,8 +407,9 @@
                 eventListenersForTarget.sort((a, b) => a.type.toLowerCase().extendedLocaleCompare(b.type.toLowerCase()));
 
                 let title = target === windowTargetIdentifier ? WI.unlocalizedString("window") : target.displayName;
+                let identifier = target === windowTargetIdentifier ? WI.unlocalizedString("window") : target.unescapedSelector;
 
-                let section = createEventListenerSection(title, eventListenersForTarget, {hideTarget: true});
+                let section = createEventListenerSection(title, eventListenersForTarget, {hideTarget: true, identifier});
                 if (target instanceof WI.DOMNode)
                     WI.bindInteractionsForNodeToElement(target, section.titleElement, {ignoreClick: true});
                 rows.push(section);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to