Bug 168518 – [Modern Media Controls] Scrubber stops moving while scrubbing on macOS

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Antoine Quint]

If you start scrubbing using the scrubber on macOS, the slider's thumb will eventually stop tracking you. This only happens when the controls are hooked into a shadow root, so maybe this is some ShadowRoot bug and not a bug in the media controls code itself.

[Radar WebKit Bug Importer]

<rdar://problem/30577637>

[Antoine Quint]

Created attachment 302444 [details]
Patch

[Dean Jackson]

Comment on attachment 302444 [details]
Patch

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

> Source/WebCore/ChangeLog:18
> +        Our solution is to make the .children setter smarter about identifying
> +        that the children list hasn't changed and that no DOM invalidation is
> +        necessary.

You need smart diffing!

> Source/WebCore/Modules/modern-media-controls/controls/layout-node.js:132
> +            for (let i = children.length - 1; i >= 0; --i) {
> +                if (children[i] !== this._children[i]) {

Shame you can't use forEach. No way to break.

[Antoine Quint]

(In reply to comment #3)
> Comment on attachment 302444 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=302444&action=review
> 
> > Source/WebCore/ChangeLog:18
> > +        Our solution is to make the .children setter smarter about identifying
> > +        that the children list hasn't changed and that no DOM invalidation is
> > +        necessary.
> 
> You need smart diffing!

I considered it but figured it wouldn't be worth the trouble.

> > Source/WebCore/Modules/modern-media-controls/controls/layout-node.js:132
> > +            for (let i = children.length - 1; i >= 0; --i) {
> > +                if (children[i] !== this._children[i]) {
> 
> Shame you can't use forEach. No way to break.

Yass.

[Antoine Quint]

Created attachment 302456 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 302456 [details]
Patch for landing

Rejecting attachment 302456 [details] from commit-queue.

New failing tests:
editing/spelling/spellcheck-async-remove-frame.html
Full output: http://webkit-queues.webkit.org/results/3175408

[WebKit Commit Bot]

Created attachment 302462 [details]
Archive of layout-test-results from webkit-cq-01 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-01  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Antoine Quint]

Committed r212869: <http://trac.webkit.org/changeset/212869>