Bug 149258 - REGRESSION: Web Inspector: DOM path bar blinks when modifying inline styles
Summary: REGRESSION: Web Inspector: DOM path bar blinks when modifying inline styles
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P1 Normal
Assignee: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-09-17 06:04 PDT by Nikita Vasilyev
Modified: 2016-05-09 18:39 PDT (History)
8 users (show)

See Also:


Attachments
[Animated GIF] Bug (76.19 KB, image/gif)
2015-09-17 06:04 PDT, Nikita Vasilyev
no flags Details
[Patch] Proposed Fix (1.79 KB, patch)
2015-10-27 14:24 PDT, Matt Baker
no flags Details | Formatted Diff | Diff
WIP (1.77 KB, patch)
2016-05-09 15:42 PDT, Nikita Vasilyev
nvasilyev: review-
nvasilyev: commit-queue-
Details | Formatted Diff | Diff
WIP (4.62 KB, patch)
2016-05-09 15:43 PDT, Nikita Vasilyev
nvasilyev: review-
nvasilyev: commit-queue-
Details | Formatted Diff | Diff
Patch (3.24 KB, patch)
2016-05-09 16:44 PDT, Nikita Vasilyev
timothy: review+
Details | Formatted Diff | Diff
Patch (6.19 KB, patch)
2016-05-09 17:25 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2015-09-17 06:04:03 PDT
Created attachment 261388 [details]
[Animated GIF] Bug

Steps:
1. Open http://www.apple.com
2. Inspect <body>
3. In the right sidebar, select "Styles — Rules"
4. Type "color" inside the Styles Attribute section

Actual:
DOM path bar blinks on every key stroke.

Expected:
DOM path bar shouldn't blink.
Comment 1 Radar WebKit Bug Importer 2015-09-17 06:04:33 PDT
<rdar://problem/22737843>
Comment 2 Timothy Hatcher 2015-09-17 08:46:47 PDT
This might be a regression caused by switching to requestAnimationFrame for updateLayoutSoon.
Comment 3 BJ Burg 2015-09-17 09:27:45 PDT
Heh, I was just going to file this bug, but my video clip got buried somewhere.
Comment 4 Matt Baker 2015-10-25 00:05:46 PDT
(In reply to comment #2)
> This might be a regression caused by switching to requestAnimationFrame for
> updateLayoutSoon.

Good call! I caused this regression in http://trac.webkit.org/changeset/188494. The flashing as the navigation bar updates is also visible when selecting nodes in the DOM tree content view.

Working on a fix.
Comment 5 Matt Baker 2015-10-27 14:24:49 PDT
Created attachment 264159 [details]
[Patch] Proposed Fix
Comment 6 WebKit Commit Bot 2015-10-27 14:43:07 PDT
Comment on attachment 264159 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 264159

Committed r191640: <http://trac.webkit.org/changeset/191640>
Comment 7 WebKit Commit Bot 2015-10-27 14:43:12 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Nikita Vasilyev 2015-12-20 01:23:42 PST
The problem still persists on r194173.

(Should I reopen bugs for cases like this?)
Comment 9 Nikita Vasilyev 2016-05-09 15:42:08 PDT
Created attachment 278447 [details]
WIP

This fixed the issue when the the DOM path bar isn't very long,
e.g. the nesting isn't very deep. I'm not sure what difference
does it make. I'm continuing looking.
Comment 10 Nikita Vasilyev 2016-05-09 15:43:13 PDT
Created attachment 278448 [details]
WIP
Comment 11 Nikita Vasilyev 2016-05-09 16:44:40 PDT
Created attachment 278459 [details]
Patch
Comment 12 Timothy Hatcher 2016-05-09 17:01:17 PDT
Comment on attachment 278459 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:10
> +        (value):

Should fix the name here.
Comment 13 Timothy Hatcher 2016-05-09 17:03:44 PDT
Comment on attachment 278459 [details]
Patch

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

>> Source/WebInspectorUI/ChangeLog:10
>> +        (value):
> 
> Should fix the name here.

Should fix the name here.

> Source/WebInspectorUI/UserInterface/Views/HierarchicalPathNavigationItem.js:77
> +    _componentsEqual(a, b)
> +    {
> +        let getRepresentedObjects = (component) => component.representedObject;
> +
> +        let representedObjectsA = a.map(getRepresentedObjects);
> +        let representedObjectsB = b.map(getRepresentedObjects);
> +        return Array.shallowEqual(representedObjectsA, representedObjectsB);
> +    }

This method has a underscore, but it is in the Public section. It should move to the Private section or, since it is only used once, inlined next to the caller above.
Comment 14 Nikita Vasilyev 2016-05-09 17:12:30 PDT
With the last patch the icons on the right are still blinking.
I need to combine my two latest patches into one.
Comment 15 Nikita Vasilyev 2016-05-09 17:25:11 PDT
Created attachment 278463 [details]
Patch

Not setting r+ since there are more changes.
Comment 16 WebKit Commit Bot 2016-05-09 18:39:39 PDT
Comment on attachment 278463 [details]
Patch

Clearing flags on attachment: 278463

Committed r200605: <http://trac.webkit.org/changeset/200605>
Comment 17 WebKit Commit Bot 2016-05-09 18:39:45 PDT
All reviewed patches have been landed.  Closing bug.