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