- Revision
- 266451
- Author
- nvasil...@apple.com
- Date
- 2020-09-01 23:57:07 -0700 (Tue, 01 Sep 2020)
Log Message
REGRESSION(r243264): Web Inspector: Style pane doesn't update after toggling CSS class
https://bugs.webkit.org/show_bug.cgi?id=202065
<rdar://problem/55149141>
Reviewed by Brian Burg.
Source/WebInspectorUI:
* UserInterface/Models/DOMNodeStyles.js:
(WI.DOMNodeStyles.prototype.refresh.fetchedMatchedStyles):
(WI.DOMNodeStyles.prototype.refresh.fetchedComputedStyle):
(WI.DOMNodeStyles.prototype.refresh):
(WI.DOMNodeStyles.prototype._parseStyleDeclarationPayload):
r243264 introduced this bug by never clearing `_styleMap` making it impossible to diff old
and new style declarations. Create and clear `_styleMap` at the same place as it was before r243264.
* UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:
(WI.SpreadsheetRulesStyleDetailsPanel):
(WI.SpreadsheetRulesStyleDetailsPanel.prototype.layout):
Layout now always re-layouts everything. Rules with modified selectors are now preserved by
exiting layout early.
(WI.SpreadsheetRulesStyleDetailsPanel.prototype._handleSectionSelectorWillChange):
Remove logic that tried to preserve indexes of CSS rules with modified selectors that
don't match (SectionIndexSymbol and everything related to it). Instead, avoid re-layout after
editing a selector.
LayoutTests:
Added a test to verify that WI.DOMNodeStyles.Event.Refreshed fires with appropriate
`significantChange` flag.
* inspector/css/node-styles-refreshed-expected.txt: Added.
* inspector/css/node-styles-refreshed.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (266450 => 266451)
--- trunk/LayoutTests/ChangeLog 2020-09-02 04:50:35 UTC (rev 266450)
+++ trunk/LayoutTests/ChangeLog 2020-09-02 06:57:07 UTC (rev 266451)
@@ -1,3 +1,17 @@
+2020-09-01 Nikita Vasilyev <nvasil...@apple.com>
+
+ REGRESSION(r243264): Web Inspector: Style pane doesn't update after toggling CSS class
+ https://bugs.webkit.org/show_bug.cgi?id=202065
+ <rdar://problem/55149141>
+
+ Reviewed by Brian Burg.
+
+ Added a test to verify that WI.DOMNodeStyles.Event.Refreshed fires with appropriate
+ `significantChange` flag.
+
+ * inspector/css/node-styles-refreshed-expected.txt: Added.
+ * inspector/css/node-styles-refreshed.html: Added.
+
2020-09-01 Yusuke Suzuki <ysuz...@apple.com>
Skip fast/css-custom-paint/out-of-memory-while-adding-worklet-module.html if Gigacage is not enabled
Added: trunk/LayoutTests/inspector/css/node-styles-refreshed-expected.txt (0 => 266451)
--- trunk/LayoutTests/inspector/css/node-styles-refreshed-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector/css/node-styles-refreshed-expected.txt 2020-09-02 06:57:07 UTC (rev 266451)
@@ -0,0 +1,16 @@
+Testing that WI.DOMNodeStyles.Event.Refreshed event dispatches with correct significantChange flag.
+
+
+== Running test suite: NodeStylesRefreshed
+-- Running test case: NodeStylesRefreshed.ClassAdded
+PASS: Adding '.baz' class should cause a significant change.
+
+-- Running test case: NodeStylesRefreshed.ClassRemoved
+PASS: Removing '.foo' class should cause a significant change.
+
+-- Running test case: NodeStylesRefreshed.IrrelevantClassAdded
+PASS: Adding '.blah' class shouldn't cause a significant change.
+
+-- Running test case: NodeStylesRefreshed.IrrelevantClassRemoved
+PASS: Removing '.blah' class shouldn't cause a significant change.
+
Added: trunk/LayoutTests/inspector/css/node-styles-refreshed.html (0 => 266451)
--- trunk/LayoutTests/inspector/css/node-styles-refreshed.html (rev 0)
+++ trunk/LayoutTests/inspector/css/node-styles-refreshed.html 2020-09-02 06:57:07 UTC (rev 266451)
@@ -0,0 +1,84 @@
+<html>
+<head>
+<script src=""
+<script>
+function test()
+{
+ let nodeStyles = null;
+ let suite = InspectorTest.createAsyncSuite("NodeStylesRefreshed");
+
+ suite.addTestCase({
+ name: "NodeStylesRefreshed.ClassAdded",
+ async test() {
+ nodeStyles.singleFireEventListener(WI.DOMNodeStyles.Event.Refreshed, (event) => {
+ InspectorTest.expectTrue(event.data.significantChange, `Adding '.baz' class should cause a significant change.`);
+ });
+
+ InspectorTest.evaluateInPage(`document.body.classList.add("baz")`);
+ await nodeStyles.refresh();
+ }
+ });
+
+ suite.addTestCase({
+ name: "NodeStylesRefreshed.ClassRemoved",
+ async test() {
+ nodeStyles.singleFireEventListener(WI.DOMNodeStyles.Event.Refreshed, (event) => {
+ InspectorTest.expectTrue(event.data.significantChange, `Removing '.foo' class should cause a significant change.`);
+ });
+
+ InspectorTest.evaluateInPage(`document.body.classList.remove("foo")`);
+ await nodeStyles.refresh();
+ }
+ });
+
+ suite.addTestCase({
+ name: "NodeStylesRefreshed.IrrelevantClassAdded",
+ async test() {
+ nodeStyles.singleFireEventListener(WI.DOMNodeStyles.Event.Refreshed, (event) => {
+ InspectorTest.expectFalse(event.data.significantChange, `Adding '.blah' class shouldn't cause a significant change.`);
+ });
+
+ InspectorTest.evaluateInPage(`document.body.classList.add("blah")`);
+ await nodeStyles.refresh();
+ }
+ });
+
+ suite.addTestCase({
+ name: "NodeStylesRefreshed.IrrelevantClassRemoved",
+ async test() {
+ nodeStyles.singleFireEventListener(WI.DOMNodeStyles.Event.Refreshed, (event) => {
+ InspectorTest.expectFalse(event.data.significantChange, `Removing '.blah' class shouldn't cause a significant change.`);
+ });
+
+ InspectorTest.evaluateInPage(`document.body.classList.remove("blah")`);
+ await nodeStyles.refresh();
+ }
+ });
+
+ WI.domManager.requestDocument((documentNode) => {
+ documentNode.querySelector("body", (contentNodeId) => {
+ if (contentNodeId) {
+ let domNode = WI.domManager.nodeForId(contentNodeId);
+ nodeStyles = WI.cssManager.stylesForNode(domNode);
+
+ nodeStyles.refreshIfNeeded().then(function() {
+ suite.runTestCasesAndFinish();
+ });
+ } else {
+ InspectorTest.fail("DOM node not found.");
+ InspectorTest.completeTest();
+ }
+ });
+ });
+}
+</script>
+</head>
+<body _onLoad_="runTest()" class="foo bar">
+<p>Testing that WI.DOMNodeStyles.Event.Refreshed event dispatches with correct significantChange flag.</p>
+<style>
+.foo {font-size: 12px;}
+.bar {background: lightyellow;}
+.baz {color: darkgreen;}
+</style>
+</body>
+</html>
Modified: trunk/Source/WebInspectorUI/ChangeLog (266450 => 266451)
--- trunk/Source/WebInspectorUI/ChangeLog 2020-09-02 04:50:35 UTC (rev 266450)
+++ trunk/Source/WebInspectorUI/ChangeLog 2020-09-02 06:57:07 UTC (rev 266451)
@@ -1,3 +1,30 @@
+2020-09-01 Nikita Vasilyev <nvasil...@apple.com>
+
+ REGRESSION(r243264): Web Inspector: Style pane doesn't update after toggling CSS class
+ https://bugs.webkit.org/show_bug.cgi?id=202065
+ <rdar://problem/55149141>
+
+ Reviewed by Brian Burg.
+
+ * UserInterface/Models/DOMNodeStyles.js:
+ (WI.DOMNodeStyles.prototype.refresh.fetchedMatchedStyles):
+ (WI.DOMNodeStyles.prototype.refresh.fetchedComputedStyle):
+ (WI.DOMNodeStyles.prototype.refresh):
+ (WI.DOMNodeStyles.prototype._parseStyleDeclarationPayload):
+ r243264 introduced this bug by never clearing `_styleMap` making it impossible to diff old
+ and new style declarations. Create and clear `_styleMap` at the same place as it was before r243264.
+
+ * UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:
+ (WI.SpreadsheetRulesStyleDetailsPanel):
+ (WI.SpreadsheetRulesStyleDetailsPanel.prototype.layout):
+ Layout now always re-layouts everything. Rules with modified selectors are now preserved by
+ exiting layout early.
+
+ (WI.SpreadsheetRulesStyleDetailsPanel.prototype._handleSectionSelectorWillChange):
+ Remove logic that tried to preserve indexes of CSS rules with modified selectors that
+ don't match (SectionIndexSymbol and everything related to it). Instead, avoid re-layout after
+ editing a selector.
+
2020-09-01 Devin Rousso <drou...@apple.com>
Web Inspector: WebSockets should be reported as type 'websocket' in Network Tab
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js (266450 => 266451)
--- trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js 2020-09-02 04:50:35 UTC (rev 266450)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js 2020-09-02 06:57:07 UTC (rev 266451)
@@ -150,8 +150,6 @@
this._needsRefresh = false;
- let previousStylesMap = this._stylesMap.copy();
-
let fetchedMatchedStylesPromise = new WI.WrappedPromise;
let fetchedInlineStylesPromise = new WI.WrappedPromise;
let fetchedComputedStylesPromise = new WI.WrappedPromise;
@@ -189,6 +187,9 @@
pseudoElementRulesPayload = pseudoElementRulesPayload || [];
inheritedRulesPayload = inheritedRulesPayload || [];
+ this._previousStylesMap = this._stylesMap;
+ this._stylesMap = new Multimap;
+
this._matchedRules = parseRuleMatchArrayPayload(matchedRulesPayload, this._node);
this._pseudoElements.clear();
@@ -250,7 +251,8 @@
let significantChange = false;
for (let [key, styles] of this._stylesMap.sets()) {
- let previousStyles = previousStylesMap.get(key);
+ // Check if the same key exists in the previous map and has the same style objects.
+ let previousStyles = this._previousStylesMap.get(key);
if (previousStyles) {
// Some styles have selectors such that they will match with the DOM node twice (for example "::before, ::after").
// In this case a second style for a second matching may be generated and added which will cause the shallowEqual
@@ -284,7 +286,7 @@
}
if (!significantChange) {
- for (let [key, previousStyles] of previousStylesMap.sets()) {
+ for (let [key, previousStyles] of this._previousStylesMap.sets()) {
// Check if the same key exists in current map. If it does exist it was already checked for equality above.
if (this._stylesMap.has(key))
continue;
@@ -302,6 +304,7 @@
}
}
+ this._previousStylesMap = null;
this._includeUserAgentRulesOnNextRefresh = false;
this.dispatchEventToListeners(WI.DOMNodeStyles.Event.Refreshed, {significantChange});
@@ -561,7 +564,8 @@
console.assert(matchesNode || style);
if (matchesNode) {
- let existingStyles = this._stylesMap.get(mapKey);
+ console.assert(this._previousStylesMap);
+ let existingStyles = this._previousStylesMap.get(mapKey);
if (existingStyles && !style) {
for (let existingStyle of existingStyles) {
if (existingStyle.node === node && existingStyle.inherited === inherited) {
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js (266450 => 266451)
--- trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js 2020-09-02 04:50:35 UTC (rev 266450)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js 2020-09-02 06:57:07 UTC (rev 266451)
@@ -42,6 +42,7 @@
this._propertyToSelectAndHighlight = null;
this._filterText = null;
this._shouldRefreshSubviews = false;
+ this._suppressLayoutAfterSelectorChange = false;
this._emptyFilterResultsElement = WI.createMessageTextView(WI.UIString("No Results Found"));
}
@@ -219,24 +220,12 @@
this._shouldRefreshSubviews = false;
- let oldSections = this._sections.slice();
- let preservedSections = oldSections.filter((section) => {
- if (section[SpreadsheetRulesStyleDetailsPanel.SectionShowingForNodeSymbol] !== this.nodeStyles.node) {
- section[SpreadsheetRulesStyleDetailsPanel.SectionShowingForNodeSymbol] = null;
- section[SpreadsheetRulesStyleDetailsPanel.SectionIndexSymbol] = -1;
- }
- return section[SpreadsheetRulesStyleDetailsPanel.SectionShowingForNodeSymbol];
- });
+ if (this._suppressLayoutAfterSelectorChange) {
+ this._suppressLayoutAfterSelectorChange = false;
+ return;
+ }
- if (preservedSections.length) {
- for (let section of oldSections) {
- if (!preservedSections.includes(section))
- this.removeSubview(section);
- }
- for (let header of this._headerMap.values())
- header.remove();
- } else
- this.removeAllSubviews();
+ this.removeAllSubviews();
let previousStyle = null;
let currentHeader = null;
@@ -263,13 +252,7 @@
if (section.style.inherited && (!previousStyle || previousStyle.node !== section.style.node))
addHeader(WI.UIString("Inherited From", "A section of CSS rules matching an ancestor DOM node"), section.style.node);
- if (!section.isDescendantOf(this)) {
- let referenceView = this.subviews[this._sections.length];
- if (!referenceView || referenceView[SpreadsheetRulesStyleDetailsPanel.SectionIndexSymbol] === this._sections.length)
- this.addSubview(section);
- else
- this.insertSubviewBefore(section, referenceView);
- }
+ this.addSubview(section);
this._sections.push(section);
section.needsLayout();
@@ -293,10 +276,6 @@
section.startEditingRuleSelector();
addSection(section);
-
- let preservedSection = preservedSections.find((sectionToPreserve) => sectionToPreserve[SpreadsheetRulesStyleDetailsPanel.SectionIndexSymbol] === this._sections.length - 1);
- if (preservedSection)
- addSection(preservedSection);
};
let addedPseudoStyles = false;
@@ -379,13 +358,8 @@
_handleSectionSelectorWillChange(event)
{
- let section = event.target;
- section[SpreadsheetRulesStyleDetailsPanel.SectionShowingForNodeSymbol] = this.nodeStyles.node;
- section[SpreadsheetRulesStyleDetailsPanel.SectionIndexSymbol] = this._sections.indexOf(section);
- console.assert(section[SpreadsheetRulesStyleDetailsPanel.SectionIndexSymbol] >= 0);
+ this._suppressLayoutAfterSelectorChange = true;
}
};
WI.SpreadsheetRulesStyleDetailsPanel.StyleSectionSymbol = Symbol("style-section");
-WI.SpreadsheetRulesStyleDetailsPanel.SectionShowingForNodeSymbol = Symbol("style-showing-for-node");
-WI.SpreadsheetRulesStyleDetailsPanel.SectionIndexSymbol = Symbol("style-index");
