Bug 204543 – macCatalyst: REGRESSION (r251320): WebKit-native media controls do not respond to hover or click

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tim Horton]

macCatalyst: REGRESSION (r251320): WebKit-native media controls do not respond to hover or click

[Tim Horton]

Created attachment 384218 [details]
Patch

[Tim Horton]

<rdar://problem/57208621>

[Tim Horton]

Created attachment 384219 [details]
Patch

[Tim Horton]

Created attachment 384849 [details]
Patch

[Tim Horton]

Created attachment 384871 [details]
Patch

[Maciej Stachowiak]

Comment on attachment 384871 [details]
Patch

Is it feasible to make test cases for this?

[Antoine Quint]

Comment on attachment 384871 [details]
Patch

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

Very nice to see those #if !PLATFORM(IOS_FAMILY) go away!

> Source/WebCore/Modules/modern-media-controls/controls/auto-hide-controller.js:63
> +            this._mediaControls.element.addEventListener("pointerout", this);

I suppose we didn't have `touchcancel` before, but I wonder if we need to handle `pointercancel` here too.

> Source/WebCore/Modules/modern-media-controls/controls/auto-hide-controller.js:68
> +            this._mediaControls.element.removeEventListener("pointerout", this);

We add a `pointerleave` event listener that we don't seem to remove.

> Source/WebCore/Modules/modern-media-controls/controls/auto-hide-controller.js:172
> +        let disableAutoHiding = this._pointerIdentifiersPreventingAutoHide.size || this._pointerIdentifiersPreventingAutoHideForHover.size;

`const` would do here.

[Antoine Quint]

(In reply to Maciej Stachowiak from comment #6)
> Comment on attachment 384871 [details]
> Patch
> 
> Is it feasible to make test cases for this?

We already have tests for auto-hide of media controls, so we should be able to extend those to work for the macCatalyst case. However, I don't know what constraints may exist due to this being macCatalyst.

[Tim Horton]

(In reply to Antoine Quint from comment #8)
> (In reply to Maciej Stachowiak from comment #6)
> > Comment on attachment 384871 [details]
> > Patch
> > 
> > Is it feasible to make test cases for this?
> 
> We already have tests for auto-hide of media controls, so we should be able
> to extend those to work for the macCatalyst case. However, I don't know what
> constraints may exist due to this being macCatalyst.

The existing tests found a mistake I made, and we don't currently have layout testing for macCatalyst. Possible we could come up with something crazy though.

[Tim Horton]

https://trac.webkit.org/changeset/253248/webkit