Bug 202446 – Web Inspector: REGRESSION(r250149): changing CSS via the Styles details sidebar panel doesn't update the associated resource

• Component: Web Inspector
• Status: RESOLVED FIXED

[Devin Rousso]

# STEPS TO REPRODUCE:
1. inspect <https://devinrousso.com>
2. select the node matching `body > header > nav > ul > li:nth-child(1) > a`
3. modify the `display: inline-block;` property of the rule with the selector `a.roll` to be something else (e.g. `display: block;`)
4. click on the source code location link next to the selector (e.g. 'common.css:1:638`)
 => FAIL: the rule with the selector `a.roll` still shows `display: inline-block;` in the text editor
5. go back to the Elements Tab
 => FAIL: the property changed in step #3 has changed back to `display: inline-block;`
6. repeat steps #3 and #4
 => FAIL: the rule with the selector `a.roll` still shows `display: inline-block;` in the text editor
7. go back to the Elements Tab
 => PASS: the property changed in step #6 still shows `display: block;`
8. go back to the Resources/Sources Tab
9. edit the same property as step #3/#6 (it should show as `display: inline-block;` right now) to a different value (e.g. `display: flex;`)
10. go back to the Elements Tab
 => PASS: the property now shows `display: flex;` and any further edits (e.g. step #3/#6) correctly propagate to the associated resource

[Devin Rousso]

I'm guessing that this is related to:
 - <https://webkit.org/b/202000> Web Inspector: Remove BranchManager in favor of just using currentRevision

[Devin Rousso]

Created attachment 380027 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 380027 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380027&action=review

r=me. Layout Test failures don't appear to be caused by this.

> Source/WebInspectorUI/UserInterface/Models/SourceCode.js:227
> +        if (this._currentRevision === this._originalRevision)
> +            this.currentRevision = this._originalRevision.copy();

Hmm, should we just do this in the constructor (and LocalResource constructor)? I tried to lazy avoid the copy, but if they share the same string it ins't really a "copy" so we would probably be fine.

[Devin Rousso]

Comment on attachment 380027 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380027&action=review

>> Source/WebInspectorUI/UserInterface/Models/SourceCode.js:227
>> +            this.currentRevision = this._originalRevision.copy();
> 
> Hmm, should we just do this in the constructor (and LocalResource constructor)? I tried to lazy avoid the copy, but if they share the same string it ins't really a "copy" so we would probably be fine.

I don't think we would want to do this in the `constructor`, due to issues in all cases other than this bug, as we wouldn't have set the `content` of the `_originalRevision`.  This isn't a "problem" right now though, as we don't really make use of `_originalRevision` (which would be through `WI.Revision.prototype.revert()`).

This is actually more of a "problem" for style sheets if you follow the same steps as described in this bug, in that the `content` of the `_originalRevision` would never get set (style sheets override `revisionForRequestedContent` to be the `_currentRevision`, so we never even touch the `_originalRevision`).

[WebKit Commit Bot]

Comment on attachment 380027 [details]
Patch

Clearing flags on attachment: 380027

Committed r250618: <https://trac.webkit.org/changeset/250618>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/55918288>