Bug 202446 - Web Inspector: REGRESSION(r250149): changing CSS via the Styles details sidebar panel doesn't update the associated resource
Summary: Web Inspector: REGRESSION(r250149): changing CSS via the Styles details sideb...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: 202000
Blocks:
  Show dependency treegraph
 
Reported: 2019-10-01 21:44 PDT by Devin Rousso
Modified: 2019-10-02 11:50 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.82 KB, patch)
2019-10-02 08:53 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-10-01 21:44:59 PDT
# 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
Comment 1 Devin Rousso 2019-10-01 21:46:27 PDT
I'm guessing that this is related to:
 - <https://webkit.org/b/202000> Web Inspector: Remove BranchManager in favor of just using currentRevision
Comment 2 Devin Rousso 2019-10-02 08:53:24 PDT
Created attachment 380027 [details]
Patch
Comment 3 Joseph Pecoraro 2019-10-02 10:39:43 PDT
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.
Comment 4 Devin Rousso 2019-10-02 11:05:52 PDT
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`).
Comment 5 WebKit Commit Bot 2019-10-02 11:49:59 PDT
Comment on attachment 380027 [details]
Patch

Clearing flags on attachment: 380027

Committed r250618: <https://trac.webkit.org/changeset/250618>
Comment 6 WebKit Commit Bot 2019-10-02 11:50:00 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2019-10-02 11:50:16 PDT
<rdar://problem/55918288>