1637908 - v-slider of vuetify is hard to interact with

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[katharina-king]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:76.0) Gecko/20100101 Firefox/76.0

Steps to reproduce:

The slider (v-silder of vuetify) can't be moved with drag-and-drop. You can test that with the sliders under this link:
https://vuetifyjs.com/en/components/sliders/

Browsers:

• Firefox on Android 68.8.0 (newest version) it doesn't work.
• Firefox on Desktop 76.0.1 all works fine.
• Chrome, Opera, Safari on all devices the sliders work.

Actual results:

The slider can't be moved with drag-and-drop. The slider can only be moved with clicks. That is a problem for range-sliders, because it moves randomly one end of the sliders. You can't select the end you want to move!

Expected results:

The circle should move to the place where we drop it

Comment 1

[katharina-king]

See comment on the github of vuetify. Vuetify thinks its a mozilla bug:
https://github.com/vuetifyjs/vuetify/issues/11409

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Would you mind giving this a try on Firefox Preview (Fenix) ? You can find it on the Google Play Store at https://play.google.com/store/apps/details?id=org.mozilla.fenix

Comment 3

[katharina-king]

Installed the newest Firefox Preview on 'Google Pixel 2'. The app either isn't responding (Close, Wait, Send Feedback) or the connection times out.
I use Android V10 (Last security update 5. Mai 2020)

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I'm sorry you had a bad experience with Firefox Preview. Would you mind trying it out on "Firefox Preview Nightly" ?

Comment 5

[Kevin Brosnan [Ex-Mozilla]]

I see the broken behavior on Nightly. If this is our bug then I expect it would live in Core::DOM: Events or DOM: UI Events & Focus Handling

Comment 6

[Kevin Brosnan [Ex-Mozilla]]

Now that I've had more time to look at this. My expectations were wrong. You can't drag in the middle of the slider to start the move. You need to tap on the thumb. Firefox is slightly harder to grab the thumb. Your tap needs to be fairly close to dead on where as on chrome if the tap circle even comes close to overlapping the thumb then the event is sent to the element. Near as I can tell the behavior is the same on ESR 68 Fennec as Fenix Nightly builds.

• Firefox behavior
• Chrome behavior

Comment 7

[katharina-king]

Yes, you are right, you can drag-and-drop the slider with Android firefox, but its extremely hard to grab the thumb. If I try to grab the thumb on my "Google Pixel 2 Phone" and I do my best it works once out of 10 trials. I tried some 100 times. So my impression is, on physical devices its even more tricky then on an emulation.

I try on this site:
https://app.skitourenguru.ch
(Switch in the Menu to EN, Click on Switzerland, go to the right panel, open the filter panel).

Comment 8

[katharina-king]

Alternatively you can try as well on the web site of Vuetify. Here you can be sure the slider is correctly implemented and up-to-date:
https://vuetifyjs.com/en/components/sliders/

Comment 9

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

Yeah this does seem like something we should look into. I poked around a bit using devtools but it's not obvious to me why this would be hard to click on, as the touchstart listener seems to be on a large-ish element.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

The touchstart listener is on a 0x0 element. But it contains an element with a bigger size inside. Maybe we should consider the overflow rect for the purposes of fluffing? (if we don't already)

Comment 11

[Timothy Nikkel (:tnikkel)]

The PositionedEventTargeting.cpp code looks like it looks at the regular frame rect in GetClosest.

Comment 12

[katharina-king]

Request to raise the priority:
Every slider of every vue.js project in the world don't work properly. Vue.js is the new trend in web dev. Does this issue not deserve a higher priority?
Just try out the sliders on vue/vuetify:
https://vuetifyjs.com/en/components/sliders/

Comment 13

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

Thanks for the ping. I do think this issue needs attention. I'll try to dig into it a bit this week or next.

I looked at the page again (in desktop devtools) and yes, the touchstart listener is on a 0x0 element and it contains a position:absolute item which I guess is out of flow and doesn't count towards the size of element. But still, with fluffing that 0x0 element should be getting expanded by the fluff margins and so it shouldn't be so hard to hit. Possibly there's something else on the page that is causing us to disable fluffing for that scenario.

Comment 14

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

I took a quick look using an android emulator and GVE. The problem is that fluffing is not enabled for touch events, only for generated mouse events. i.e. if you do a tap, that will generate a mousemove, mousedown, mouseup sequence, and all of those events have fluffing applied. But the source touch events do not, and the vue.js slider is listening for touch events. If I flip the ui.touch.radius.enabled pref in about:config then fluffling is turned on for touch events as well and it's easier to grab the slider.

So the fluffing code itself is working as intended, but fluffing is disabled on touch events and that's making it hard to trigger the touch listener in this case. Maybe we should consider turning on touch event fluffing by default, but probably use a much narrower set of heuristics as to what is targetable by touch events - like only if it has touch listeners, rather than anything "clickable".

Comment 15

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

I wrote patches that do the above, and it mostly works, but APZ hit-testing still uses the original target point. That means preventDefault() may be ignored. In particular on the vue.js slider, it means that you can drag the slider if you move horizontally but the page still pans if you move vertically, because APZ doesn't wait for the content listener to run and do the preventDefault(). I don't know if there's a good way to handle that without perf implications.

But arguably this is still an improvement.

Comment 16

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

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=9172a5519a1adb4ed98ffc3c2f0cbd85d31286dc

Comment 17

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

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=515ca493ca38ec7292666f7abbb06b4d319b0095 now with test

Comment 18

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

Attachment: Bug 1637908 - Move the clickableAncestor computation further down. r?snorp

There's a few early-return conditions where we don't need to do this computation
at all. So this patch just moves the computation to after the early-returns. No
functional changes intended.

Comment 19

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

Attachment: Bug 1637908 - Minor cleanup. r?snorp

Depends on D91025

Comment 20

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

Attachment: Bug 1637908 - Introduce a touchable heuristic and use it for retargeting touch events. r?snorp

The existing heuristics in GetClickableAncestor have evolved over time to be
quite specific to mouse/click events, because those are the ones that we have
enabled by default. The heuristics we want for touch events are different and
should generally be very conservative, because we almost never want to retarget
the raw touch events. This patch introduces a new GetTouchableAncestor
function that implements this new conservative heuristic function and uses it
for determining the retargeting destination for touch events.

Depends on D91026

Comment 21

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

Attachment: Bug 1637908 - Add some tests for the touchable heuristic to the existing event radius tests. r?snorp

Depends on D91027

Comment 22

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

Attachment: Bug 1637908 - Enable touch event retargeting on android with new touchable heuristic. r?snorp

Depends on D91028

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52300d600d62
Move the clickableAncestor computation further down. r=snorp
https://hg.mozilla.org/integration/autoland/rev/f9a938192f7b
Minor cleanup. r=snorp
https://hg.mozilla.org/integration/autoland/rev/7ec2d3c9f951
Introduce a touchable heuristic and use it for retargeting touch events. r=snorp
https://hg.mozilla.org/integration/autoland/rev/5ad03a51bb6d
Add some tests for the touchable heuristic to the existing event radius tests. r=snorp
https://hg.mozilla.org/integration/autoland/rev/251445d07289
Enable touch event retargeting on android with new touchable heuristic. r=snorp

Comment 24

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/52300d600d62
https://hg.mozilla.org/mozilla-central/rev/f9a938192f7b
https://hg.mozilla.org/mozilla-central/rev/7ec2d3c9f951
https://hg.mozilla.org/mozilla-central/rev/5ad03a51bb6d
https://hg.mozilla.org/mozilla-central/rev/251445d07289

Comment 25

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

This would probably benefit from baking in Nightly a bit before I consider uplifting.

Comment 26

[katharina-king]

Yesterday my Google Pixel 2 updated to the 83.1.0 and the sliders WORK perfectly. Thats great, thank you.

Comment 27

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

Thanks for confirming!