Bug 148019 – Web Inspector: Highlight DOM node attribute changes in parallel, not sequentially

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

Created attachment 258986 [details]
[Animated GIF] Bug

Steps:
1. Open http://n12v.com/focus-transition-2/
2. Inspect the animated GIF
3. Click on the animated GIF

Expected:
Both "width" and "class" attributes are highlighted for the same duration.

Actual:
First "width" get highlighted for a fraction of a second (far less than 1s), then "class". Both attributes are never highlighted at the same time. On the attached GIF I changed the animation duration to 10s to exaggerate the effect.

[Radar WebKit Bug Importer]

<rdar://problem/22283189>

[Radar WebKit Bug Importer]

<rdar://problem/22283207>

[Devin Rousso]

Created attachment 259024 [details]
Patch

[Devin Rousso]

Created attachment 259031 [details]
Patch

[Blaze Burg]

Comment on attachment 259024 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:3
> +        Web Inspector: Highlight DOM node attribute changes in parallel, not consequent

I fixed the bug title grammar, please ditto.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:1470
> +            for (let i = 0; i < this._nodeStateChanges; ++i) {

It makes me sad we have to duplicate our helper Array.prototype.remove but we can't do much better until arrows land.

Maybe one day it could be: this._nodeStateChanges.removeIf((node) => element === node.element);

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:1477
> +        this._boundNodeChangedAnimationEnd = animationEnd.bind(this);

Should only need to bind this once, right?

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:1486
>              element.classList.add("node-state-changed");

There (used to be) a bug where adding and removing a class would not cause the animation to restart, so we would have to use delayedWork. Is this no longer the case? Do new animations interrupt old ones?

[Devin Rousso]

Created attachment 259033 [details]
Patch

[Blaze Burg]

Comment on attachment 259033 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 259033 [details]
Patch

Clearing flags on attachment: 259033

Committed r188496: <http://trac.webkit.org/changeset/188496>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.