[REGRESSION] Toggling Outset/Inset in DevTools box-shadow tool doesn't update all elements |
|||||||||||||
Issue descriptionChrome 57.0.2979.0 OSX 10.12.2 What steps will reproduce the problem? (1) Visit https://badssl.com/ (2) Inspect one of the links. (3) Go to the selector for `#links a` (4) Click on the purple double-box to edit a box-shadow (5) Change the blur and spread to around 15px each, for visibility. (6) Click "Inset", then "Outset" What is the expected result? All affected link elements switch back to an outset box-shadow. What happens instead? Some of the affected elements are stuck on inset. Toggling Inset/Out again leaves the elements inconsistent.
,
Jan 31 2017
The bisect range: https://chromium.googlesource.com/chromium/src/+log/29942ba33818a9db2fc90a441b80d56cbdbc1f1f..90d4ea3d543f0031769b3aacac2d3e084b95fb7d Looks like the https://codereview.chromium.org/2557533005 is the offender. @rune, could you please take a look? This is crucial for the devtools functionality.
,
Jan 31 2017
I'm only able to change one of the links changing the shadow in the inspector. Only the link I inspected is modified. If I modify the class of one of the other links, the style is updated. Some missing style invalidation somewhere.
,
Feb 1 2017
It turns out this is not related to DevTools - it's a CSSOM issue. DevTools rely on CSSOM for editing and for this reason expose the issue. The simplified step-by-step: 1. Goto badssl.com 2. Run the following code in the console: var link = document.querySelector('a.bad') getMatchedCSSRules(link)[0].style.setProperty('border', '2px solid red') Expected: all links on the page get red border. Actual: only the first link gets the red border.
,
Feb 1 2017
,
Feb 1 2017
,
Feb 1 2017
I've found the issue. I did a quick and dirty fix to confirm it's a missing clearance of the matched properties cache. I'm @blinkon today and then travelling, so I might not be able to submit a proper fix before the weekend.
,
Feb 1 2017
@rune: thank you for the update!
,
Feb 2 2017
Is this change applicable to only Mac or any others OSes too?
,
Feb 2 2017
All platforms which rely on Blink are affected (so everything but iOS)
,
Feb 6 2017
For review: https://codereview.chromium.org/2679623002/
,
Feb 7 2017
Perfect, thank you! Eagerly waiting for this to land.
,
Feb 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8c00af2b243ead60e3d7011b5a85367735f19ba1 commit 8c00af2b243ead60e3d7011b5a85367735f19ba1 Author: rune <rune@opera.com> Date: Wed Feb 08 09:59:42 2017 Clear MatchedPropertiesCache on StyleRule changes. CSSOM changes used to cause a FullStyleUpdate which cleared the whole StyleResolver. With the new active stylesheet update using RuleSet- based style invalidation, clearing the MatchedPropertiesCache was missing. The reason it needs to be cleared, is that the hash key for the cache entry is based on StylePropertySet pointers which don't change when adding/removing declarations to a mutable StylePropertySet. R=meade@chromium.org BUG= 681183 Review-Url: https://codereview.chromium.org/2679623002 Cr-Commit-Position: refs/heads/master@{#448939} [modify] https://crrev.com/8c00af2b243ead60e3d7011b5a85367735f19ba1/third_party/WebKit/Source/core/css/CSSStyleSheet.cpp [modify] https://crrev.com/8c00af2b243ead60e3d7011b5a85367735f19ba1/third_party/WebKit/Source/core/dom/StyleEngineTest.cpp
,
Feb 8 2017
,
Feb 8 2017
[Auto-generated comment by a script] We noticed that this issue is targeted for M-57; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-57 label, otherwise remove Merge-TBD label. Thanks.
,
Feb 9 2017
,
Feb 9 2017
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a77dc542a2c90e257328aec3e6efb0e334a69413 commit a77dc542a2c90e257328aec3e6efb0e334a69413 Author: Rune Lillesveen <rune@opera.com> Date: Thu Feb 09 13:28:55 2017 Clear MatchedPropertiesCache on StyleRule changes. CSSOM changes used to cause a FullStyleUpdate which cleared the whole StyleResolver. With the new active stylesheet update using RuleSet- based style invalidation, clearing the MatchedPropertiesCache was missing. The reason it needs to be cleared, is that the hash key for the cache entry is based on StylePropertySet pointers which don't change when adding/removing declarations to a mutable StylePropertySet. R=meade@chromium.org BUG= 681183 Review-Url: https://codereview.chromium.org/2679623002 Cr-Commit-Position: refs/heads/master@{#448939} (cherry picked from commit 8c00af2b243ead60e3d7011b5a85367735f19ba1) Review-Url: https://codereview.chromium.org/2685093003 . Cr-Commit-Position: refs/branch-heads/2987@{#404} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/a77dc542a2c90e257328aec3e6efb0e334a69413/third_party/WebKit/Source/core/css/CSSStyleSheet.cpp [modify] https://crrev.com/a77dc542a2c90e257328aec3e6efb0e334a69413/third_party/WebKit/Source/core/dom/StyleEngineTest.cpp
,
Feb 13 2017
,
Feb 15 2017
Verified on Mac os 10.12.2 , ubuntu 14.04 and windows 7 using chrome beta M57 #57.0.2987.54 and issue is fixed. Steps Followed : 1. Navigated to https://badssl.com/ and inspected one of the links 2. Clicked on purple double-box and edited the blur and spread values to 15px and 16px respectively. 3. Clicked on Inset and outset and observed no stuck on elements and tool updated all elements. Attached screencast for reference. Adding TE-Verified Labels. Thanks! |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by eostroukhov@chromium.org
, Jan 30 2017