1688888 - Audio scrubber on sputniknews.com is draggable outside of the area it should be

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Dennis Schubert [:denschub]]

Attachment: Screen recording showing the touch point and scrubber behavior

The audio scrubber in this sputniknews.com article shows a bit of a weird behavior.

As you can see in the attached screen recording, you can drag the scrubber element far below where it should be. In this case, it causes an issue where you can't always hit the play button and end up seeking forward instead. This reproduces in Fenix and GVE, but not on Desktop.

Also, this is probably in the wrong component, so please move it around if that's the case.

Comment 1

[Dennis Schubert [:denschub]]

Attachment: Devtools visualization of that scrubber element

Also attaching a screenshot of the element's area according to the devtools - so this is probably not an issue with the element's sizing, but something else.

Comment 2

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

So is the event listener for some touch event added to ancestor element of the 'scrubber'?

Or, I wonder if some of this https://searchfox.org/mozilla-central/rev/69babd862de70cabfa1d0a369d38e4881bd41e4d/layout/base/PositionedEventTargeting.cpp#198,214 affects here.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Yeah, I suspect this is just the "fat fingers" target enlargement code... Dennis, does this repro on Fenix with ui.touch.radius.enabled=false?

Comment 4

[Dennis Schubert [:denschub]]

This does not reproduce with ui.touch.radius.enabled=false, so yeah, this is "fat finger" code. One day, I'll remember that this preference exists... :/

Unsure what to do here, now. We could try reach out to the site, but given this works fine in Chrome, I think we'd have a hard time getting them into making the player larger/more compatible.

Comment 5

[Jonathan Kew [:jfkthame]]

Sounds like this is the ui.touch.radius.* options working as expected, isn't it? Or is the behavior somehow incorrect even once that is taken into account?

Comment 6

[Dennis Schubert [:denschub]]

(In reply to Jonathan Kew (:jfkthame) from comment #5)

Or is the behavior somehow incorrect even once that is taken into account?

I'm honestly not quite sure what to reply here.

It is probably working as expected, yeah, but maybe the expectations are wrong and maybe there is something we can do here. I don't know specifics on how the touch radius is implemented and if there is room for additional logic. What I do know is that the media player in this issue is pretty much unusable for Firefox users, but works fine in Chrome and Safari.

Since this is probably not the only instance of a player similar to this, I'd hate to close this as a wontfix, because it might drive users away. On the other hand, I honestly know nothing about the touch radius implementation, so I don't know if we can do something here.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Putting bug 1618532 in "Regressed by" since I believe this issue has started since the bug. Though I am not sure why the scrubber is targeted rather than the play/pause buttons, we should improve the event retargetting machinery. I will check what's going on there in this case later.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

I just quickly dumped event.retarget log and IIUC the log, it looks the event retarggeting machinery correctly targeted to the play/pause buttons.

D/event.retarget Final target is 0x76ed5918834

And the 0x76ed5918834 frame is;

Block(a)(1)@76ed59188340 parent=76ed5af77f88 (x=1200, y=960, w=660, h=600) [content=76ed5b728a80] [cs=76ed59125f28] <

I believe this is either the pause button or the play button. I am pretty sure it's not the scrubber which is a <div> element. So something goes wrong after we found the target.

Clearing NI to me since further investigation needs more time.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Setting S3 for now since users can avoid this situation by zooming up the content.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Note that mouse events and touch events can both get retargeted (separately). Per comment 4, it's the touch retargeting that's causing this issue, so this is more likely a regression from bug 1637908 than from bug 1618532. And I don't know if comment 8 relates to retargeting of touch events or mouse events; if it's mouse events it may be irrelevant.

Also keep in mind that bug 1637908 was explicitly intended to make these kinds of scrubbers easier to drag. But we could reduce the radius mm values to fine-tune the behaviour some, and I expect that would probably be good enough for this case.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Thank you kats for the input! What I tested in comment 8 is on an Android emulator by using a mouse, I believe it behaves touch events, no? (I confirmed this issue happened at that time)

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #10)

Also keep in mind that bug 1637908 was explicitly intended to make these kinds of scrubbers easier to drag. But we could reduce the radius mm values to fine-tune the behaviour some, and I expect that would probably be good enough for this case.

Thanks! Let's do it!

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I started to look into this. I modified the ui.touch.radius.topmm preference to change the behavior. It defaults to 5mm and I tried:

• at 2mm, I almost never accidentally move the audio scrubber
• at 3mm, I sometimes accidentally move the audio scrubber but it's straightforward to me to move my finger

I would go with 2mm which matches the default bottom radius (left/right is at 3mm). I tested this behavior on vuetify js sliders via https://vuetifyjs.com/en/components/sliders/ (from bug 1637908 which enabled the feature that caused this behavior) and they are still easy to use. However, I'm not sure how the change in this preference would impact other pages with touch event areas. My intuition says it'll be okay – in bug 1637908, the issue was touch event fluffing was not enabled at all, not that the fluffing radius was incorrect so having a smaller value should still address the issue and similar ones – but I don't know for sure.

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Bug 1688888: decrease ui.*.radius.topmm on Android. r=botond

On one website, the touch input fluffing made an audio scrubber so large that it
covered other playback controls, making them inaccessible. This patch addresses
this problem by changing the top fluffing radius so it more closely matches the
bottom radius.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aa1e0da89303
decrease ui.*.radius.topmm on Android. r=botond

Comment 15

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/aa1e0da89303

Comment 16

[Pascal Chevrel:pascalc (PTO until April 26)]

Mike, is that something we would like to uplift in 93 or can it ride the 94 train? Thanks

Comment 17

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

This bug has been around for a while and it's particularly difficult for me as an individual to understand it works well for all users on all pages so I'd rather let it ride the 94 train – thanks for asking.