Bug 199760 - Web Inspector: Changes: dismissing a blank property makes style rule to show in Changes panel
Summary: Web Inspector: Changes: dismissing a blank property makes style rule to show ...
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: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-07-12 14:42 PDT by Nikita Vasilyev
Modified: 2019-07-12 16:24 PDT (History)
4 users (show)

See Also:

Attachments
[Animated GIF] Bug (43.99 KB, image/gif)
2019-07-12 14:42 PDT, Nikita Vasilyev 		no flags Details
Patch (1.41 KB, patch)
2019-07-12 14:46 PDT, Nikita Vasilyev 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2019-07-12 14:42:40 PDT 
Created attachment 374036 [details]
[Animated GIF] Bug

Steps:
1. Inspect <body> (or any element)
2. In the Styles panel, click to add a new CSS property
3. Click elsewhere to dismiss it
4. Select Changes panel

Expected:
Changes panel is empty

Actual:
Changes panel shows one CSS rule. All text is gray; no additions or deletions.
Comment 1 Nikita Vasilyev 2019-07-12 14:46:23 PDT 
Created attachment 374038 [details]
Patch
Comment 2 Devin Rousso 2019-07-12 14:47:59 PDT 
Comment on attachment 374038 [details]
Patch

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

rs=me

> Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:443
> +                this._ownerStyle.updatePropertiesModifiedState();

Why not move this call inside `shiftPropertiesAfter`?  That way, you could remove the `updatePropertiesModifiedState` on <480 as well.
Comment 3 Nikita Vasilyev 2019-07-12 14:55:42 PDT 
Comment on attachment 374038 [details]
Patch

(In reply to Devin Rousso from comment #2)
> Comment on attachment 374038 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=374038&action=review
> 
> rs=me
> 
> > Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:443
> > +                this._ownerStyle.updatePropertiesModifiedState();
> 
> Why not move this call inside `shiftPropertiesAfter`?  That way, you could
> remove the `updatePropertiesModifiedState` on <480 as well.

While it would save one line of code, I think it's more clear when shiftPropertiesAfter only shifts properties.
Comment 4 WebKit Commit Bot 2019-07-12 16:21:38 PDT 
Comment on attachment 374038 [details]
Patch

Clearing flags on attachment: 374038

Committed r247406: <https://trac.webkit.org/changeset/247406>
Comment 5 WebKit Commit Bot 2019-07-12 16:21:39 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 6 Radar WebKit Bug Importer 2019-07-12 16:24:06 PDT 
<rdar://problem/53032783>