Closed Bug 1177842 Opened 5 years ago Closed 4 years ago

[Aries][TaskManager] Card View is very jumpy scrolling betweens tabs; requires very precise touch to navigate single tab

Categories

(Firefox OS Graveyard :: Gaia::System::Task Manager, defect, P3)

ARM
Gonk (Firefox OS)
defect

Tracking

(firefox44 affected, b2g-v2.2 unaffected, b2g-v2.5 fixed, b2g-master affected)

RESOLVED WONTFIX
FxOS-S10 (30Oct)
Tracking Status
firefox44 --- affected
b2g-v2.2 --- unaffected
b2g-v2.5 --- fixed
b2g-master --- affected

People

(Reporter: onelson, Unassigned)

References

()

Details

(Keywords: regression, Whiteboard: [3.0-Daily-Testing], [Spark][systemsfe])

Attachments

(2 files, 2 obsolete files)

Description:
When the user is navigating cards in the task manager (multiple applications open), they will observe that the cards will react very abrubtly to the users slide gestures. This makes it more difficult to move between single cards at a time.
Related issue, with 2.2 and 3.0, appears that you cannot slide more than one card at a time. Having both as options may be beneficial by gauging the slide based on users speed/length of slide, with an easier bottom (single card) to achieve.

PreReq:
* open 5+ applications (number isn't dependent, just multiple)
Repro Steps:
1) Update a Aries to 20150626120208
2) Hold home button to open card view on apps
3) Slide finger from left-to-right and right-to-left
4) Observe sliding transition of cards

Actual:
Cards move very quickly on single slide gesture, requires additional precision to move single card at a time

Expected:
Cards move dependent on speed/length of slide; easy to slide single or multiple cards at a time


Environmental Variables:
---------------------------

Device: Aries 3.0
Build ID: 20150626120208
Gaia: 8a1e4ae522c121c5cacd39b20a5386ec9055db82
Gecko: 56e207dbb3bd
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 41.0a1 (3.0)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

******************************************
Difficult to observe this issue on 3.0 due to bug 1162535 and bug 1172167

Device: Flame 3.0
BuildID: 20150624160209
Gaia: eb0d4aefa62b20420d6fa0642515a110daca5d97
Gecko: 7b0df70e27ea
Gonk: a4f6f31d1fe213ac935ca8ede7d05e47324101a4
Version: 41.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0
**************************

Issue DOES NOT REPRO on 2.2 for flame devices
Results: Cards move one at a time when introducing a slide gesture in TaskManager; could be modified to allow multiple card slide

Device: Flame 2.2
BuildID: 20150626002504
Gaia: 1f8981d7872e3c0053571c26fb3edaf401844d75
Gecko: 2f8b845e5fa3
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
------------------------------------


Repro frequency: 5/5
See attached: 
video
Inaccurate tracking flag on 2.2
This is an Aries specific issue.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
[Blocking Requested - why for this release]:
We've regressed in behavior/performance here from flame.
blocking-b2g: --- → 2.5?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga)
Keywords: regression
Jet, we probably need some fine tuning here.
Flags: needinfo?(bugs)
blocking-b2g: 2.5? → 2.5+
These are the tune-able prefs for modifying scroll snapping behavior:
https://bugzilla.mozilla.org/attachment.cgi?id=8569537&action=diff

Sam, I don't have an Aries but will go looking for one. If you've already got one, please see if you can repro the bug and fiddle with the prefs above.
Flags: needinfo?(bugs) → needinfo?(sfoster)
Ok I had a fiddle, of the two prefs, layout.css.scroll-snap.prediction-sensitivity looks most interesting. I'm restarting after adjusting the value but THB I'm not seeing much difference: I've dialled it way down to < 0.1, and up to 1.0 and I still find it too easy to swipe past the next card. But I've spent a lot of time with the task manager at this point so I might be the wrong person to tune this - I've retrained my swipes so my experience might not be representative. Am I doing it wrong (I'm setting a value for the pref in webIDE, then restarting b2g) or maybe I'm misunderstanding the expected value range? If you can confirm, I can find someone else to take a look.
Flags: needinfo?(sfoster) → needinfo?(bugs)
Hi Sam,

Reducing layout.css.scroll-snap.prediction-sensitivity should generally cause the nearer snap point to be selected.  I'm not familiar with WebIDE; however, I use a bash script to quickly experiment with various preferences:

#!/bin/bash

PREFS_JS=$(adb shell echo -n "/data/b2g/mozilla/*.default")/prefs.js
adb pull $PREFS_JS
vim prefs.js
adb shell stop b2g
adb push prefs.js $PREFS_JS
adb shell start b2g
Also, I'd suggest that 0.1 be too short a value.  If you need to set the value less than 0.5 seconds or so, I'd recommend reducing the layout.css.scroll-snap.prediction-max-velocity preference instead.  You may find that there will be no difference when changing layout.css.scroll-snap.prediction-max-velocity in small increments, until you reach a threshold that clamps the distance based on the speed of the flinging motion.
Flags: needinfo?(bugs) → needinfo?(sfoster)
Duplicate of this bug: 1186303
There's a video on the most recent dupe that illustrates this problem: https://www.youtube.com/watch?v=rihp8rGquqg

I tried again editing those prefs and again wasn't really able to discern a difference in behavior with the values I put in (using the shell script to edit and restart b2g) so I've suggested :mcav sit down with :jet at some point to review
Flags: needinfo?(sfoster)
Jet/Sam, 

There hasn't been any activity on this 2.5 blocker. Looking at comment 9, NI'ed to bring it up on your list.
Flags: needinfo?(sfoster)
Flags: needinfo?(bugs)
Redirecting to :mcav
Flags: needinfo?(sfoster)
Marcus, see Comment #9
Flags: needinfo?(m)
Duplicate of this bug: 1206963
Whiteboard: [3.0-Daily-Testing], [Spark] → [3.0-Daily-Testing], [Spark][systemsfe]
I think our issue here is the current default behaviour of snapping. It feels like it's calculating where you'd end up, then picking the nearest snap point, where as really, we want to pick the closest snap point *before* where you'd end up. I also think some extra friction on top of this change of behaviour when snapping is enabled may also be desired.

cc'ing Botond, as it's an APZC bug and he may be able to help anyone that has the time to take this on.
(In reply to Chris Lord [:cwiiis] from comment #14)
> It feels like it's calculating where you'd end up, then picking the nearest
> snap point, where as really, we want to pick the closest snap point *before*
> where you'd end up.

Let me make sure I understand your proposal correctly.

Say we have the following:

S   |    |      |     X   | 
    A    B      C         D

where A, B, C, and D are snap points, S is your starting position, and X is where a fling would end up without snapping. Are you suggesting that, while right now we might snap to D (because it's closer to X than C), we should snap to C instead?
(In reply to Botond Ballo [:botond] from comment #15)
> (In reply to Chris Lord [:cwiiis] from comment #14)
> > It feels like it's calculating where you'd end up, then picking the nearest
> > snap point, where as really, we want to pick the closest snap point *before*
> > where you'd end up.
> 
> Let me make sure I understand your proposal correctly.
> 
> Say we have the following:
> 
> S   |    |      |     X   | 
>     A    B      C         D
> 
> where A, B, C, and D are snap points, S is your starting position, and X is
> where a fling would end up without snapping. Are you suggesting that, while
> right now we might snap to D (because it's closer to X than C), we should
> snap to C instead?

Exactly :) (plus also some extra friction) My rationale is that the page should never accelerate after you lift your finger, which is how this ends up feeling (relative to other scrolling)
OK, thanks for clarifying! That seems like a reasonable suggestion, and it seems like the scroll snapping spec gives us the flexibility to tweak things like this.

I'm not very familiar with the code that chooses a snap point based on a predicted destination, but I know it's located in ScrollFrameHelper::GetSnapPointForDestination() [1] , so looking at that should be a good starting point for someone who'd like to implement this adjustment. Kip, as the one who wrote this code, can probably provide more specific guidance.

[1] https://dxr.mozilla.org/mozilla-central/rev/607a236c229994df99766c005f9ec729532d7747/layout/generic/nsGfxScrollFrame.cpp#3599
(In reply to Botond Ballo [:botond] from comment #17)
> I'm not very familiar with the code that chooses a snap point based on a
> predicted destination, but I know it's located in
> ScrollFrameHelper::GetSnapPointForDestination() [1] , so looking at that
> should be a good starting point for someone who'd like to implement this
> adjustment. Kip, as the one who wrote this code, can probably provide more
> specific guidance.
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/rev/
> 607a236c229994df99766c005f9ec729532d7747/layout/generic/nsGfxScrollFrame.
> cpp#3599

Just been having a look at this - We'd need a way to get an indicator of what initiated the snap here - we don't want to change the behaviour when using the scrollbar or mousewheel, this is really only for flinging. Looking to see how involved this will be...
Actually, on further investigation, I think it may be desirable to have this behaviour always, regardless of whether it was a fling that initiated the scroll or not (except for when an absolute scroll position was requested, of course).
The first part of my suggestion was much easier than expected :)

It still feels too jumpy after this patch, I think (also as I suggested) that there needs to be extra friction as well. This can be done at the APZC level though, perhaps as a multiplier for the destination value?
Attachment #8673668 - Flags: review?(kgilbert)
Comment on attachment 8673668 [details] [diff] [review]
0001-Bug-1177842-Preserve-context-when-scroll-snapping-fr.patch

Actually, this doesn't seem to be working as intended, the behaviour is asymmetric... Looking into it.
Attachment #8673668 - Flags: review?(kgilbert)
Flags: needinfo?(m)
Chris, should this be assigned to you or are we holding off for a more definitive solution? Just want to make sure the unassigned blockers are still intentionally unassigned.
Flags: needinfo?(chrislord.net)
(In reply to Marcus Cavanaugh [:mcav] (Mozilla SF) from comment #22)
> Chris, should this be assigned to you or are we holding off for a more
> definitive solution? Just want to make sure the unassigned blockers are
> still intentionally unassigned.

I've been wanting to get to it, but purposefully haven't assigned because I have other, higher priority tasks to deal with. Whether I can get to it soon depends on how home screen blockers go, however. It would be good to get some platform assistance on these scroll-snapping issues.

My last findings were that my suggestion in comment #14 might not be actionable because scrolling is async but the snap point is fetched content-side, so I think really we just want to add extra friction when snapping.
Flags: needinfo?(chrislord.net)
(In reply to Chris Lord [:cwiiis] from comment #23)
> (In reply to Marcus Cavanaugh [:mcav] (Mozilla SF) from comment #22)
> > Chris, should this be assigned to you or are we holding off for a more
> > definitive solution? Just want to make sure the unassigned blockers are
> > still intentionally unassigned.
> 
> I've been wanting to get to it, but purposefully haven't assigned because I
> have other, higher priority tasks to deal with. Whether I can get to it soon
> depends on how home screen blockers go, however. It would be good to get
> some platform assistance on these scroll-snapping issues.
> 
> My last findings were that my suggestion in comment #14 might not be
> actionable because scrolling is async but the snap point is fetched
> content-side, so I think really we just want to add extra friction when
> snapping.

Do you mean that even if you change the content-side code to pick an earlier snap point, with current friction settings the compositor can have scrolled past it by the time content notifies the compositor of the snap point? Doesn't this still cause scrolling back to the content-chosen snap point?
Priority: -- → P3
(In reply to Botond Ballo [:botond] (at standards meeting Oct 19-24) from comment #24)
> Do you mean that even if you change the content-side code to pick an earlier
> snap point, with current friction settings the compositor can have scrolled
> past it by the time content notifies the compositor of the snap point?
> Doesn't this still cause scrolling back to the content-chosen snap point?

This is right - especially in the task manager, this frequently leads to you scrolling past 2 snap points, and snapping to the first, which felt very unnatural. I think the easiest thing to do here immediately is just apply a greater friction for snapping end-point prediction.
Attachment #8673668 - Attachment is obsolete: true
Flagging for early review so that we can get it committed quickly if/when we need to.

This value seems ok, but I'll seek UX approval before committing.

What feels odd now is that the deceleration can be quite sudden (you can still scroll 2, 3 even 4 cards in the task manager if you're trying, but a casual fling will fling no more than 1 or 2). It almost feels like we want the spring effect when snapping, but let's keep it simple for now.

I think we could do much better here if the apzc was knowledgeable of snap points along an axis, rather than leaving it to content.
Attachment #8676851 - Flags: review?(botond)
try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=75803f8e591d

When this completes, could UX give this a go and provide feedback on whether this is better/worse / whether we should tweak some more? The pref is 'apz.fling_snap_friction', but it isn't live, so tweaks will require restarts.
Flags: needinfo?(rmacdonald)
Comment on attachment 8676851 [details] [diff] [review]
0001-Bug-1177842-Introduce-separate-friction-for-fling-sn.patch

Review of attachment 8676851 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the requested change. Thanks!

::: b2g/app/b2g.js
@@ +1022,4 @@
>  pref("apz.fling_curve_function_y2", "1.0");
>  pref("apz.fling_curve_threshold_inches_per_ms", "0.01");
>  pref("apz.fling_friction", "0.0019");
> +pref("apz.fling_snap_friction", "0.01");

Please add the pref to modules/libpref/init/all.js as well.
Attachment #8676851 - Flags: review?(botond) → review+
Assignee: nobody → chrislord.net
Comment addressed, carrying r+ - small tweak to value that feels better to me.
Attachment #8676851 - Attachment is obsolete: true
Flags: needinfo?(rmacdonald)
Flags: needinfo?(bugs)
Attachment #8678987 - Flags: review+
This seems to resolve the primary issue of being too sensitive. I can now scroll in "smaller increments."

It feels however as if my scroll action has little effect on the device. E.g., I do what I think is a very long swipe and only move 2 cards. I do a small swipe and move 1-2 cards. So it does not feel like there is a relationship between my physical movement and the device's behavior. When I do manage to make the device scroll more than a few cards at a time, I do not know what I did differently to cause that. 

Not sure if this existed before or not: Move to the card to the farthest left. Scroll a very small amount as if you want Card #2 to move into the primary slot.
What happens: Card #2 bounces in (slightly) then bounces back to its original position so 1st card is still primary. 
Expected: "Move forward" 2nd card is in the primary slot. Maybe this is an issue to be resolved post 2.5...
Unfortunately there's not much we can do in terms of simple changes to remedy this :(

Currently snap points are dealt with in the Gecko thread and not on the compositor - so when you fling a scrollable view, the compositor (the bit handling input and drawing to the screen) asks the content thread (the bit rendering the web page, running JavaScript, etc.) to snap to the nearest scroll point to a predicted end-point. The content thread can take an arbitrary amount of time to respond to this request, and this time is often far less than the ideal of 'instant'. Once the compositor gets the response, it will smoothly animate to that snap point.

This patch artificially shortens that predicted end-point with an alternative friction value to be applied, but otherwise doesn't change anything - the result is that there's now a strong disconnect between the action and the result (relative to non-snap-point-scrolling), although the result is at least now a bit more user-friendly.

Ideally, I think we want snap points to be dealt with entirely on the compositor, and in addition to that, to be dealt with differently - I think that a better experience would be if snap points were treated as 'magnetic' points on an axis and the simulation changed accordingly. This would feel more natural and look better. If we wanted to make it harder to scroll past snap points, we'd increase that magnetism, instead of having a different friction value for containers using snap points.

This is a pretty big job though, and very much post-2.5.
https://hg.mozilla.org/mozilla-central/rev/ed2fdc36728d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S10 (30Oct)
Duplicate of this bug: 1180167
ftr, I'm not really happy with the 'fix' I've checked in here, but I guess it will have to do for now.

I find with the setting I checked in, switching between horizontal panels on the home screen is now harder than it should be (and forget about the vertical home screen paging), but lower values made the task manager still reasonably tricky to use.

I think this really depends on bug 1219296 for a proper fix.
This backs out attachment #8768987 [details]. After living with it for some time, this is a worse experience than the sensitive card view and negatively affects all other scroll-snapping call-sites.

We need a more comprehensive fix in apz for this.
Attachment #8682569 - Flags: review?(botond)
Gregor, I think we should back this out as the experience is considerably worse all-in-all (especially panning between the two home-screen panels). I think the blocker status here should be reconsidered. Either we increase friction for all scrolling to help here, or we punt until bug 1219296 is fixed and we can implement something better in apz.
Flags: needinfo?(anygregor)
Hmm, while I still think reverting this change is a good move, snapping between home-screen panels appears to have regressed from some other change.
(In reply to Chris Lord [:cwiiis] from comment #38)
> Hmm, while I still think reverting this change is a good move, snapping
> between home-screen panels appears to have regressed from some other change.

Filed bug 1221186 for this.
Let's reopen the bug if we're going to be landing another patch here.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #8682569 - Flags: review?(botond) → review+
Got the ok from Gregor, pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/5dec2ebb1e71

Unblocking 2.5 on this, but let's continue to use this bug to track the real solution once bug 1219296 lands.
blocking-b2g: 2.5+ → ---
Flags: needinfo?(anygregor)
Keywords: leave-open
Depends on: 1219296
Comment on attachment 8682569 [details] [diff] [review]
backout-bug1177842

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Janky behaviour with css scroll snapping
Testing completed: Manual testing completed
Risk to taking this patch (and alternatives if risky): Basically zero, this backs out a very last-minute patch.
String or UUID changes made by this patch: None
Attachment #8682569 - Flags: approval‑mozilla‑b2g44?
Unassigning for now until the dependent bug is fixed.
Assignee: chrislord.net → nobody
Comment on attachment 8682569 [details] [diff] [review]
backout-bug1177842

Approved for 2.5

Thanks
Attachment #8682569 - Flags: approval‑mozilla‑b2g44? → approval‑mozilla‑b2g44+
Significant improvements have been made to scroll snapping in bug 1219296, which may have fixed part of all of the underlying issue of this bug. The specific STR here are B2G-specific, so I'm going to close this bug as WONTFIX for now.
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → WONTFIX
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.