Skip to content

Commit

Permalink
Cherry-pick 261883@main (96c03ed). https://bugs.webkit.org/show_bug.c…
Browse files Browse the repository at this point in the history
…gi?id=253956

    Web Inspector: Styles Panel: Adding a new CSS rule doesn't work on first attempt
    https://bugs.webkit.org/show_bug.cgi?id=253956

    Reviewed by Patrick Angle.

    To add a new CSS rule, an inspector stylesheet is first created.
    This operation triggers a `DOMNodeStyles.refresh()`.

    The operation to add the new rule to this stylesheet also triggers a
    refresh. But if the promise for the first refresh is still pending,
    the second call is ignored. This means that the Styles panel ends up
    not showing any matching styles from the initially empty stylehseet.

    A subsequent call to add a new rule bypasses the need to create a new
    stylesheet. The `DOMNodeStyles.refresh()` it trigges brings the Styles
    panel in sync with the latest contents of the inspector-generated stylehseet.
    That's why two rules show up on the first successful attempt.

    The logic to mark the newly created rule's selector as editable is too lenient:
    it marks all CSS rules with a selector identical to the one of the newly created rule.
    This patch makes a stricter check by identifying the newly created rule by its `CSSStyle.styleId`.

    * Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js:
    (WI.DOMNodeStyles.prototype.addRule.completed):
    (WI.DOMNodeStyles.prototype.addRule.addedRule):
    * Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:
    (WI.SpreadsheetRulesStyleDetailsPanel):
    (WI.SpreadsheetRulesStyleDetailsPanel.prototype.spreadsheetCSSStyleDeclarationSectionAddNewRule):
    (WI.SpreadsheetRulesStyleDetailsPanel.prototype.layout):
    (WI.SpreadsheetRulesStyleDetailsPanel.prototype._addNewRule):
    (WI.SpreadsheetRulesStyleDetailsPanel.prototype._addedNewRuleCallback):

    Canonical link: https://commits.webkit.org/261883@main
  • Loading branch information
rcaliman-apple authored and aperezdc committed Apr 6, 2023
1 parent 79b2722 commit e4b5fe0
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
18 changes: 16 additions & 2 deletions Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
Expand Up @@ -362,12 +362,20 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
{
selector = selector || this._node.appropriateSelectorFor(true);

let result = new WI.WrappedPromise;
let target = WI.assumingMainTarget();

function completed()
{
target.DOMAgent.markUndoableState();
this.refresh();

// Wait for the refresh promise caused by injecting an empty inspector stylesheet to resolve
// (another call will be ignored while one is still pending),
// then refresh again to get the latest matching styles which include the newly created rule.
if (this._pendingRefreshTask)
this._pendingRefreshTask.then(this.refresh.bind(this));
else
this.refresh();
}

function styleChanged(error, stylePayload)
Expand All @@ -380,8 +388,12 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object

function addedRule(error, rulePayload)
{
if (error)
if (error){
result.reject(error);
return;
}

result.resolve(rulePayload);

if (!text || !text.length) {
completed.call(this);
Expand All @@ -403,6 +415,8 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
inspectorStyleSheetAvailable.call(this, WI.cssManager.styleSheetForIdentifier(styleSheetId));
else
WI.cssManager.preferredInspectorStyleSheetForFrame(this._node.frame, inspectorStyleSheetAvailable.bind(this));

return result.promise;
}

effectivePropertyForName(name)
Expand Down
Expand Up @@ -38,6 +38,7 @@ WI.SpreadsheetRulesStyleDetailsPanel = class SpreadsheetRulesStyleDetailsPanel e
this._headerMap = new Map;
this._sections = [];
this._newRuleSelector = null;
this._newRuleStyleId = null;
this._ruleMediaAndInherticanceList = [];
this._propertyToSelectAndHighlight = null;
this._filterText = null;
Expand Down Expand Up @@ -210,7 +211,7 @@ WI.SpreadsheetRulesStyleDetailsPanel = class SpreadsheetRulesStyleDetailsPanel e
spreadsheetCSSStyleDeclarationSectionAddNewRule(section, selector, text)
{
this._newRuleSelector = selector;
this.nodeStyles.addRule(this._newRuleSelector, text);
this.nodeStyles.addRule(this._newRuleSelector, text).then((rulePayload) => {this._newRuleStyleId = rulePayload.style.styleId;});
}

spreadsheetCSSStyleDeclarationSectionSetAllPropertyVisibilityMode(section, propertyVisibilityMode)
Expand Down Expand Up @@ -280,7 +281,7 @@ WI.SpreadsheetRulesStyleDetailsPanel = class SpreadsheetRulesStyleDetailsPanel e
style[SpreadsheetRulesStyleDetailsPanel.StyleSectionSymbol] = section;
}

if (this._newRuleSelector === style.selectorText && style.enabledProperties.length === 0)
if (this._newRuleSelector === style.selectorText && style.enabledProperties.length === 0 && Object.shallowEqual(this._newRuleStyleId, style.id))
section.startEditingRuleSelector();

addSection(section);
Expand Down Expand Up @@ -327,6 +328,7 @@ WI.SpreadsheetRulesStyleDetailsPanel = class SpreadsheetRulesStyleDetailsPanel e
addPseudoStyles();

this._newRuleSelector = null;
this._newRuleStyleId = null;

this.element.append(this._emptyFilterResultsElement);

Expand Down Expand Up @@ -359,7 +361,7 @@ WI.SpreadsheetRulesStyleDetailsPanel = class SpreadsheetRulesStyleDetailsPanel e
this._newRuleSelector = this.nodeStyles.node.appropriateSelectorFor(justSelector);

const text = "";
this.nodeStyles.addRule(this._newRuleSelector, text, stylesheetId);
this.nodeStyles.addRule(this._newRuleSelector, text, stylesheetId).then((rulePayload) => {this._newRuleStyleId = rulePayload.style.styleId;});
}

_handleSectionSelectorOrGroupingWillChange(event)
Expand Down

0 comments on commit e4b5fe0

Please sign in to comment.