Title: [252682] trunk
- Revision
- 252682
- Author
- drou...@apple.com
- Date
- 2019-11-19 20:31:20 -0800 (Tue, 19 Nov 2019)
Log Message
Web Inspector: DOM.highlightSelector should work for "div, div::before"
https://bugs.webkit.org/show_bug.cgi?id=204306
Reviewed by Brian Burg.
.:
* ManualTests/inspector/overlay-selectors.html: Added.
Source/WebCore:
In r252436, the implementation of `DOM.highlightSelector` was changed from just calling
`document.querySelectorAll` to actually attempting to mimic what the CSS selector matching
engine does. Basically, this meant adding logic to walk the entire DOM tree and for each
node test each `CSSSelector` of the given `selector` string to see if it matched.
At the time, I had incorrectly assumed that once a selector was found that matched the
current node, it wouldn't need to be checked against ever again. This would be a fine
assumption if we didn't care about `:before`/`:after`, but since `DOM.highlightSelector`
also wants to match those, it is necessary to test every `CSSSelector` in case a later one
in the given `selector` string matches a pseudo-element (e.g. `div, div:before`).
The fix is simply to change `break` to `continue` and to ensure that every item in the
generated `NodeList` is unique (otherwise the overlay for a node may be drawn twice).
* inspector/agents/InspectorDOMAgent.cpp:
(WebCore::InspectorDOMAgent::highlightSelector):
Modified Paths
Added Paths
Diff
Modified: trunk/ChangeLog (252681 => 252682)
--- trunk/ChangeLog 2019-11-20 04:10:24 UTC (rev 252681)
+++ trunk/ChangeLog 2019-11-20 04:31:20 UTC (rev 252682)
@@ -1,3 +1,12 @@
+2019-11-19 Devin Rousso <drou...@apple.com>
+
+ Web Inspector: DOM.highlightSelector should work for "div, div::before"
+ https://bugs.webkit.org/show_bug.cgi?id=204306
+
+ Reviewed by Brian Burg.
+
+ * ManualTests/inspector/overlay-selectors.html: Added.
+
2019-11-12 Carlos Alberto Lopez Perez <clo...@igalia.com>
[GTK][WPE] Support Pointer Events
Added: trunk/ManualTests/inspector/overlay-selectors.html (0 => 252682)
--- trunk/ManualTests/inspector/overlay-selectors.html (rev 0)
+++ trunk/ManualTests/inspector/overlay-selectors.html 2019-11-20 04:31:20 UTC (rev 252682)
@@ -0,0 +1,44 @@
+<style>
+p {
+ position: fixed;
+ right: 20px;
+ bottom: 20px;
+ margin: 0;
+}
+
+.expected {
+ position: absolute;
+ width: 300px;
+ height: 100px;
+ background-image: linear-gradient(to right, rgba(111, 168, 220, 0.66) 33.3%, transparent 33.3%, transparent 66.6%, rgba(111, 168, 220, 0.66) 66.6%);
+}
+.actual, .actual::before {
+ display: inline-block;
+ width: 100px;
+ height: 100px;
+}
+.actual {
+ position: relative;
+}
+.actual::before {
+ position: absolute;
+ left: 200px;
+ content: "";
+}
+</style>
+<body>
+ <div id="test1">
+ <div class="expected" hidden></div>
+ <div class="actual"></div>
+ </div>
+
+ <p>Inspect this page and hover the `<code>.actual, .actual::before</code>` CSS selector in the Styles panel of the details sidebar in the Elements tab. Click <button>Show Expected</button> to compare with the expected result.</p>
+ <script>
+let showingExpected = false;
+document.querySelector("button").addEventListener("click", (event) => {
+ showingExpected = !showingExpected;
+ for (let node of document.querySelectorAll(".expected"))
+ node.hidden = !showingExpected;
+});
+ </script>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (252681 => 252682)
--- trunk/Source/WebCore/ChangeLog 2019-11-20 04:10:24 UTC (rev 252681)
+++ trunk/Source/WebCore/ChangeLog 2019-11-20 04:31:20 UTC (rev 252682)
@@ -1,3 +1,27 @@
+2019-11-19 Devin Rousso <drou...@apple.com>
+
+ Web Inspector: DOM.highlightSelector should work for "div, div::before"
+ https://bugs.webkit.org/show_bug.cgi?id=204306
+
+ Reviewed by Brian Burg.
+
+ In r252436, the implementation of `DOM.highlightSelector` was changed from just calling
+ `document.querySelectorAll` to actually attempting to mimic what the CSS selector matching
+ engine does. Basically, this meant adding logic to walk the entire DOM tree and for each
+ node test each `CSSSelector` of the given `selector` string to see if it matched.
+
+ At the time, I had incorrectly assumed that once a selector was found that matched the
+ current node, it wouldn't need to be checked against ever again. This would be a fine
+ assumption if we didn't care about `:before`/`:after`, but since `DOM.highlightSelector`
+ also wants to match those, it is necessary to test every `CSSSelector` in case a later one
+ in the given `selector` string matches a pseudo-element (e.g. `div, div:before`).
+
+ The fix is simply to change `break` to `continue` and to ensure that every item in the
+ generated `NodeList` is unique (otherwise the overlay for a node may be drawn twice).
+
+ * inspector/agents/InspectorDOMAgent.cpp:
+ (WebCore::InspectorDOMAgent::highlightSelector):
+
2019-11-19 Youenn Fablet <you...@apple.com>
getUserMedia echoCancellation constraint has no affect
Modified: trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp (252681 => 252682)
--- trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp 2019-11-20 04:10:24 UTC (rev 252681)
+++ trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp 2019-11-20 04:31:20 UTC (rev 252682)
@@ -1260,7 +1260,8 @@
SelectorChecker selectorChecker(*document);
- Vector<Ref<Node>> nodes;
+ Vector<Ref<Node>> nodeList;
+ HashSet<Node*> seenNodes;
for (auto& descendant : composedTreeDescendants(*document)) {
if (!is<Element>(descendant))
@@ -1281,8 +1282,8 @@
unsigned ignoredSpecificity;
if (selectorChecker.match(*selector, descendantElement, context, ignoredSpecificity)) {
- nodes.append(descendantElement);
- break;
+ if (seenNodes.add(&descendantElement))
+ nodeList.append(descendantElement);
}
if (context.pseudoIDSet) {
@@ -1290,25 +1291,29 @@
if (pseudoIDs.has(PseudoId::Before)) {
pseudoIDs.remove(PseudoId::Before);
- if (auto* beforePseudoElement = descendantElement.beforePseudoElement())
- nodes.append(*beforePseudoElement);
+ if (auto* beforePseudoElement = descendantElement.beforePseudoElement()) {
+ if (seenNodes.add(beforePseudoElement))
+ nodeList.append(*beforePseudoElement);
+ }
}
if (pseudoIDs.has(PseudoId::After)) {
pseudoIDs.remove(PseudoId::After);
- if (auto* afterPseudoElement = descendantElement.afterPseudoElement())
- nodes.append(*afterPseudoElement);
+ if (auto* afterPseudoElement = descendantElement.afterPseudoElement()) {
+ if (seenNodes.add(afterPseudoElement))
+ nodeList.append(*afterPseudoElement);
+ }
}
if (pseudoIDs) {
- nodes.append(descendantElement);
- break;
+ if (seenNodes.add(&descendantElement))
+ nodeList.append(descendantElement);
}
}
}
}
- m_overlay->highlightNodeList(StaticNodeList::create(WTFMove(nodes)), *highlightConfig);
+ m_overlay->highlightNodeList(StaticNodeList::create(WTFMove(nodeList)), *highlightConfig);
}
void InspectorDOMAgent::highlightNode(ErrorString& errorString, const JSON::Object& highlightInspectorObject, const int* nodeId, const String* objectId)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
