Too easy to accidentally close tabs when scrolling vertically
Categories
(Firefox for Android :: Tabs, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox136 | --- | verified |
People
(Reporter: certt4fz, Assigned: 007)
References
(Blocks 1 open bug, Regressed 3 open bugs)
Details
(Whiteboard: [fxdroid][group4])
Attachments
(3 files)
Steps to reproduce:
I have noticed this change very recently. Could be since the last update to 127.
I have the tabs in list view and when I wanted to scroll vertically with my thumb I always accidentally close a tab if my swiping motion is even a little bit horizontal. This is really annoying and it definitely wasn't as sensitive before. I need to be very careful and slow when scrolling the tab list now.
Perhaps related to this bug which is marked as fixed: https://bugzilla.mozilla.org/show_bug.cgi?id=1854335
Actual results:
Tabs get closed by accident when scrolling through tabs in list view.
Expected results:
Should be able to scroll vertically through the tab list without being afraid of it being misinterpreted as a horizontal "close tabs" swipe. This worked fine in the past.
Comment 1•1 year ago
|
||
Can confirm -- happens to me all the time now. I'm using the Grid view so it's not limited to the List view.
Thank goodness for the 'Undo' banner that slides in at the bottom, but sometimes I don't notice in time to get my tab back π
Comment 2•1 year ago
|
||
Perhaps related to this bug which is marked as fixed: https://bugzilla.mozilla.org/show_bug.cgi?id=1854335
bug 1854335 was supposed to fix this problem, not make it worse. Noah: is this still known as a problem?
Assignee | ||
Comment 3•1 year ago
|
||
Hey Sugar, thanks for filing this bug. I've confirmed that some accidental tab swipes are occurring when trying to vertically scroll through the Tabs Tray. I've prioritized this bug for my team to investigate this further.
Updated•1 year ago
|
Comment 4•1 year ago
|
||
This issue is a recurring complaint in the Play Store reviews. We really need to fix it.
Is this just a matter of finding a better (higher) value for VELOCITY_THRESHOLD_DP
?
Assignee | ||
Comment 5•1 year ago
|
||
(In reply to Markus Stange [:mstange] from comment #4)
This issue is a recurring complaint in the Play Store reviews. We really need to fix it.
Is this just a matter of finding a better (higher) value for
VELOCITY_THRESHOLD_DP
?
I am able to reproduce this with some very slow swipes, so I don't believe so. The velocity threshold is a parameter for defining what's the minimum swipe speed that indicates a "swipe to dismiss" has been triggered.
For some reason, the underlying Android component is triggering the drag gesture (when it's specified for horizontal drags) even when the gesture has a large degree of Y-axis movement. I'm looking into this further to see if this can be manually configured.
Comment 6•1 year ago
•
|
||
Thanks for looking into this!
Here's the implementation of the threshold detection: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/AnchoredDraggable.kt;l=1398-1432;drc=64c19e2612ecaca5c53a3f7b96d7b4fec2991709
It gets the velocity in onDragStopped by calling Velocity.toFloat: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/AnchoredDraggable.kt;l=430;drc=64c19e2612ecaca5c53a3f7b96d7b4fec2991709
And Velocity.toFloat is aware of the scroll direction, and gets the correct directional component: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/Scrollable.kt;l=606;drc=01d017d5f115f8ff9cb1555b6e25daf651ac7b71
So I think our threshold of 150 dp per second might just be really really low. You can cross a lot of distance if you move your finger across the screen for a full second, even at a slow speed.
Assignee | ||
Comment 7•1 year ago
|
||
Thanks for finding that Android source code! I'll play around with some velocity thresholds to see if it at least minimizes part of the issue
Comment 8•1 year ago
|
||
Oh, I think I see what you are saying now.
A diagonal drag over a tab in the tabs tray can have the following outcomes:
- The tab is swiped away horizontally, no vertical scrolling is performed.
- The tab is swiped a bit horizontally but bounces back (remains open), no vertical scrolling is performed.
- The tab is not moved, only vertical scrolling is performed.
What I'm suggesting would turn more cases of 1 into 2. But you're saying you'd like 3 to happen in more cases. And I think I agree.
So for this we'd want the AnchoredDraggable view to detect that the scroll direction (within the touch slop) is more vertical than horizontal, and we'd want it to pass off scrolling to the outer scroll view. I found this issue which sounds similar (though it is about ViewPager2 rather than about AnchoredDraggable):
First, the horizontal slide of ViewPager2 is detected in the onInterceptTouchEvent method of the RecyclerView class, right?
And the criterion is whether the touch distance in the X direction is greater than mTouchSlop.
I think we should add to this criterion whether the touch distance in the X direction is greater than the touch distance in the Y direction.
I also found https://developer.android.com/develop/ui/compose/touch-input/pointer-input/scroll#nested-scrolling but haven't spent enough time with it to fully understand it.
Assignee | ||
Comment 9•1 year ago
•
|
||
(In reply to Markus Stange [:mstange] from comment #8)
A diagonal drag over a tab in the tabs tray can have the following outcomes:
- The tab is swiped away horizontally, no vertical scrolling is performed.
- The tab is swiped a bit horizontally but bounces back (remains open), no vertical scrolling is performed.
- The tab is not moved, only vertical scrolling is performed.
What I'm suggesting would turn more cases of 1 into 2. But you're saying you'd like 3 to happen in more cases. And I think I agree.
Ah gotcha. Then yeah, increasing the velocity threshold might reduce the scenario where flings to scroll the tabs tray are triggering swipe to dismiss flings. Interestingly though, Google's default in their samples and a component (noted below) is only 125
So for this we'd want the AnchoredDraggable view to detect that the scroll direction (within the touch slop) is more vertical than horizontal, and we'd want it to pass off scrolling to the outer scroll view. I found this issue which sounds similar (though it is about ViewPager2 rather than about AnchoredDraggable):
I also found https://developer.android.com/develop/ui/compose/touch-input/pointer-input/scroll#nested-scrolling but haven't spent enough time with it to fully understand it.
Good find on that Google Issue. Yes, that's exactly what I've been looking into: is it possible to manually ignore swipes within a certain y-axis threshold.
Much to my dismay, the Modifier.anchoredDraggable
we leverage simply looks for a non-zero x-axis drag delta and declares this as a drag occurring, regardless of whatever the y delta is. It's also not clear how it is differentiating between scroll and drag gestures in this scenario.
I think I've ran into a wall though. After investigating a few hours yesterday and today, I've tried the following:
- Pass-in a
InteractionSource
toModifier.anchoredDraggable
- The
InteractionSource
provides no drag measurement data. It simply returns aFlow
ofInteraction
events that we can listen to.
- The
- Listen to drag gestures via
detectDragGestures
withinModifier.pointerInput
pointerInput
consumes any drag gestures, so even trying to log drags preventsModifier.anchoredDraggable
from being triggered. Similarly, if I place this Modifier beforeanchoredDraggable
, it doesn't get called because the drag handling has been consumed downstream.
- Replace our in-house
SwipeToDismissBox
(which utilizesModifier.anchoredDraggable
) with the Material 3SwipeToDismissBox
- Even the official Google component has the exact same bug.
My educated assumption is that a nested scroll isn't at play here, but it might be worth investigating a little.
From there, the only approach I have not tried is completely rewriting our SwipeToDismissBox
from scratch to use a custom Modifier.anchoredDraggable
that would be able to be less sensitive to consuming drag gestures with a "large" y-axis delta.
SwipeToDismissBox
was already a rewrite from earlier this year, as Google deprecated the SwipeToDismiss
Composable in favor of Modifier.anchoredDraggable
, so trying to handle the drag gesture within a scrolling parent in Compose has been fraught with development churn.
Comment 10•11 months ago
|
||
Is it possible to add setting to disable swipe-to-delete behaviour? It causes issue for more than a year making browsing tabs very annoying, but there is literally no reason in it while there is also X-mark button that does just the same thing.
Comment 11•11 months ago
|
||
(In reply to Dmitriy Nikiforov from comment #10)
Is it possible to add setting to disable swipe-to-delete behaviour? It causes issue for more than a year making browsing tabs very annoying, but there is literally no reason in it while there is also X-mark button that does just the same thing.
I made one: https://imgur.com/a/fenix-disable-swipable-tabs-kEVbgEW
Assignee | ||
Comment 12•9 months ago
|
||
I found another google issue tracker for this problem. I'm also going to propose another round of investigation of this for my team's next sprint
Comment 13•8 months ago
|
||
Could we just write our own swipe detection logic, to decouple ourselves from Google fixing this?
Assignee | ||
Comment 14•8 months ago
|
||
Yes, and my team has it in our current sprint to start working towards that!! Stay tuned here for more updates!
The google issue from #12 has a potential starting point for this investigation.
Assignee | ||
Updated•8 months ago
|
Assignee | ||
Updated•8 months ago
|
Assignee | ||
Comment 16•8 months ago
|
||
Updated•7 months ago
|
Comment 19•7 months ago
|
||
I want to mention that in Thunderbird seems like there is no problem with accidental swiping, but it also has special configuration with options to disable swipe actions completely: https://i.imgur.com/zlYtDRr.jpeg
Comment 20•7 months ago
|
||
Assignee | ||
Comment 21•7 months ago
|
||
I've landed a patch for Nightly only with a rewritten drag (swipe to close) gesture. It should be available in tonight's or tomorrow's Nightly build to try out.
We'll let this bake to gather some feedback, address any bugs, and fine tune anything we need before letting this ride the trains to Release
Comment 22•7 months ago
|
||
bugherder |
Assignee | ||
Comment 24•7 months ago
|
||
QE, can you take a look at the swipe gesture when swiping to close a tab in the Tabs Tray?
The patch that landed yesterday is to make it so vertical scrolls do not accidentally trigger the swipe gesture (and therefore accidental tab closures). This change will only be available in Nightly, and we'll want to vet out the new swipe gesture and squash any more bugs before considering letting the fix ride the trains to Release.
Comment 25•7 months ago
|
||
When you drag the tab, it doesn't go over the adjacent tab, it stays stable on the tab, when you keep your finger on the tab and we move it left, right.
Comment 26•7 months ago
|
||
(In reply to eclaudiu64 from comment #25)
Created attachment 9460380 [details]
mobizen_20250118_123505.mp4When you drag the tab, it doesn't go over the adjacent tab, it stays stable on the tab, when you keep your finger on the tab and we move it left, right.
And as an addition, when we hold the tab in the other direction and put the other tabs on top and then swipe, the tab closes, kind of like the error was.
Assignee | ||
Updated•7 months ago
|
Comment 27•7 months ago
|
||
Verified the swipe to close gesture in Nightly 136.0a1 from 01/22 and indeed the issues mentioned above as regressions are reproducible on our side as well.
Apart from that:
- I was able to reproduce the issue Claudiu mentioned in Comment 26, on all latest releases and filed bug 1943007 for it.
- I saw another issue, in which the selected tab was displayed under the first or last tab while manually reordering tabs in tabs tray, and filed bug 1943008 for it. This might be a regression, since I could not reproduce it yet on other releases.
I'll leave the qe-needed flag to further test this once the regressions are fixed.
Comment 28•7 months ago
|
||
Thanks for testing Delia! After Bug 1942735 lands, can you please see if the issue is still reproducible?
Comment 29•6 months ago
|
||
Verified as fixed in the latest Nightly 136.0a1 from 01/27. Vertical scrolls no longer trigger the swipe gesture and deletion of tabs.
Tested with Nothing Phone (2a) 5G (Android 14), OPPO A15s (Android 10) and Google Pixel 8 Pro (Android 14).
Updated•6 months ago
|
Description
•