Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 84150ea85574551811c8802fe27fa252bc571d31
      
https://github.com/WebKit/WebKit/commit/84150ea85574551811c8802fe27fa252bc571d31
  Author: Razvan Caliman <[email protected]>
  Date:   2023-03-20 (Mon, 20 Mar 2023)

  Changed paths:
    M 
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js
    M Source/WebInspectorUI/UserInterface/Views/SpreadsheetRuleHeaderField.js

  Log Message:
  -----------
  Web Inspector: New CSS property unexpectedly dropped from empty CSS rule when 
tabbing through or editing selector
https://bugs.webkit.org/show_bug.cgi?id=245768

Reviewed by Patrick Angle.

There are two issues here:
1) changing the selector of an empty CSS rule causes the first added CSS 
property to be dropped.
2) tabbing through the editable text field for a CSS selector marks it as 
changed even when it's not.

The core problem is the order of operations.

Tabbing out of a CSS selector text field first calls
`SpreadsheetCSSStyleDeclarationSection.spreadsheetRuleHeaderFieldWillNavigate()`
from the `SpreadsheetRuleHeaderField._handleKeyDown` handler.

For an empty CSS rule, this creates a new blank CSS property with:
`SpreadsheetCSSStyleDeclarationEditor.startEditingFirstProperty()` -> 
`CSSStyleDeclaration.newBlankProperty()`.

At this point, the frontend model for the CSS rule contains a 
`CSSStyleDeclaration` with one `CSSProperty`.

In quick succession, the second operation happens when the selector text field 
loses focus:
`SpreadsheetCSSStyleDeclarationSection.spreadsheetRuleHeaderFieldDidCommit()`
from the `SpreadsheetRuleHeaderField._handleBlur` handler.

If the selector text has changed, a request is sent to the backend via 
`DOMNodeStyles.changeRuleSelector()`.
A request is then made for the latest matching styles via 
`DOMNodeStyles.refresh()`.

For an empty CSS rule, there are no matching styles. As a result, the 
`CSSStyleDeclaration` of the corresponding
CSS rule is updated with an empty list of CSS properties in 
`DOMNodeStyles._parseStyleDeclarationPayload()`:

```
if (style) {
    style.update(text, properties, styleSheetTextRange);
    return style;
}
```

This has the effect of orphaning the new blank `CSSProperty` introduced earlier.

`CSSProperty._updateOwnerStyleText()` is called to persist the `CSSProperty` to 
the backend.
This calls `CSSStyleDeclaration.generateFormattedText()` to serialize the whole 
CSS declaration block.
But our `CSSStyleDeclaration` was just emptied of properties by the update in
`DOMNodeStyles._parseStyleDeclarationPayload()` so it falls back to returning 
an empty string:

```
let styleText = "";
...
let properties = this._styleSheetTextRange ? this.visibleProperties : 
this._properties;
...
return styleText;
```

When the `CSSProperty` commits with an empty name and value, the matching 
styles obtained
in response to `DOMNodeStyles.refresh()` are empty, and the orphaned property 
is dropped.

Trying again with the newly created blank CSS property will work because this 
time
there's no race with `DOMNodeStyles.refresh()` from the selector change.

To fix the first issue, we must ensure that the operation to update the selector
and the style refresh it causes occurs before adding a new CSS property to avoid
the style refresh invalidating the models on the frontend.

To fix the second issue, we don't call 
`SpreadsheetRuleHeaderField.stopEditing()` in the
keydown handler for Tab or Enter  because that resets `_valueBeforeEditing` 
which causes
the check in  `SpreadsheetRuleHeaderField._handleBlur` to always consider the 
selector
as having been changed, thus triggering the operations described above.

* 
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:
(WI.SpreadsheetCSSStyleDeclarationSection):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleSpreadsheetSelectorFieldDidCommit):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleSpreadsheetSelectorFieldWillNavigate):
* Source/WebInspectorUI/UserInterface/Views/SpreadsheetRuleHeaderField.js:
(WI.SpreadsheetRuleHeaderField):
(WI.SpreadsheetRuleHeaderField.prototype.stopEditing):
(WI.SpreadsheetRuleHeaderField.prototype._handleBlur):
(WI.SpreadsheetRuleHeaderField.prototype._handleKeyDown):

Canonical link: https://commits.webkit.org/261861@main


_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to