1864148 - Unable to scroll product's carousel if swiping over a video that plays on aliexpress.com

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P2)

Description

[Calin Tanase (PTO until 15th of May)]

Attachment: scroll alie.mp4

Environment:
Operating system: OnePlus 6 A6000 (Android 11)
Firefox version: Nightly 121.0a1-20231107214948

Steps to reproduce:

1. Go to any product that has videos in the carousel on aliexpress.com
2. Play the video.
3. Try scrolling the corousel by swiping over the video.

Expected Behaviour:
The carousel can be scrolled.

Actual Behaviour:
The carousel cannot be scrolled while swiping over a video that plays.

Notes:

1. Screen rec provided
2. Reproducible on Firefox Release as well
3. Not reproducible on Chrome
4. Issue found during WebCompat team [Top100] websites testing

Comment 1

[Raul Bucata (⛔PTO until May 20🌴)]

Verified this issue and still reproduces on Firefox 123 and 125.

Tested with:

Browser / Version: Firefox Release 122.1.0 (2016001831-🦎122.0.1-20240205133611🦎)/ Firefox Nightly 125.0a1 (2016004775-🦎125.0a1-20240220212334🦎)/ Firefox Beta 123.0b9 (2016002567-🦎123.0-20240209102425🦎)/ Chrome Mobile Version 121.0.6167.164
Operating System: Google Pixel 3 (Android 12) -1080 x 2160 pixels, 18:9 ratio (~443 ppi density)
Operating System: Oppo Find X5 (Android 13) - 1080 x 2400 pixels, 20:9 ratio (~402 ppi density)

Comment 2

[Ksenia Berezina [:ksenia]]

Attachment: 1864148.html

Comment 3

[Ksenia Berezina [:ksenia]]

It looks like touch events get swallowed by the video event as long as video controls are present (controls attribute is set on <video>) in Firefox. It doesn't happen in Chrome, and the site is relying on that. Not sure which behaviour is correct.

Hi Sean, wonder if you have any thoughts on this? I've attached a testcase above.

Comment 4

[Sean Feng [:sefeng211]]

Thanks Ksenia, that example is really helpful. Looks like we stops the event bubbling at https://searchfox.org/mozilla-central/rev/faa7b1b2a7b509df04a8bafbf8520fc162ad1363/dom/html/HTMLMediaElement.cpp#4859-4863, so that the parent element (which is the slider this case) can't receive those events.

I checked bug 1327097 which introduced this change. So the motivation was to trap those events within the media elements to make them handled solely by controls. However, it looks like we don't check whether the controls are visible or not.

I wonder if we should only apply these logic when the controls are visible to the user. Olli, what do you think?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

There is !this->Controls() check. But if that doesn't catch some case when controls aren't visible, then sure, we should add some extra check.

Comment 6

[Sean Feng [:sefeng211]]

Thanks.

/me working this bug now

Comment 7

[Sean Feng [:sefeng211]]

Attachment: Bug 1864148 - Make HTMLMediaElement only trap touch events when the ControlBar or the PlayButton or the ClickToPlay button is the target

Bug 1327097 introduced the original change to trap all these events
because we only wanted the controls to handle them. However, this
should only happen when controls is visible.

This patch allows those event still propagates when controlBar
is transparent.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Sean, what's next for this patch land?

Comment 9

[Sean Feng [:sefeng211]]

er I can't remember why I put it aside, I'll start finishing off the patch.

Comment 10

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Hi Sean! Do you still plan to drive this cross the completion line? I talked with Masayuki, who is willing to try helping out, if you'd like assistance from others. :)

Comment 11

[Sean Feng [:sefeng211]]

Sure! though I'll be a bit slow :)

Comment 12

[Sean Feng [:sefeng211]]

Here's what Chrome did https://codereview.chromium.org/406213002

Seems like they stop the events when the slider is the target https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/media_controls/elements/media_control_volume_slider_element.cc;l=177;drc=877c4eff05eee91a64ab498b2f02928fe718278b;bpv=1;bpt=1

and something similar for the Play button https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/media_controls/elements/media_control_overlay_play_button_element.cc;l=99;drc=877c4eff05eee91a64ab498b2f02928fe718278b;bpv=1;bpt=1

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/72e21c991048
https://hg.mozilla.org/integration/autoland/rev/919f1584d566
Make HTMLMediaElement only trap touch events when the ControlBar or the PlayButton or the ClickToPlay button is the target r=emilio,dom-core,smaug

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/55015 for changes under testing/web-platform/tests

Comment 15

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/919f1584d566

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot