- 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);