1153711 - Flinging horizontally in the B2G browser doesn't work (no kinetic scroll)

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect)

Description

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

STR:
On B2G, fire up the browser and go to http://people.mozilla.com/~kgupta/grid.html
Fling the page horizontally.

Expected:
Page continues moving horizontally after finger goes up.

Actual:
Page stops moving pretty much as soon as the finger goes up. This is annoying when trying to fling horizontally.

qawanted for branch checks.

Comment 1

[Pi Wei Cheng [:piwei] (inactive)]

This issue reproduces on Flame 3.0 and 2.2. Swiping left/right on the provided page does not give user a slow-to-stop scrolling effect, instead it just stops abruptly as soon as finger leaves screen.

Device: Flame 3.0 Master (full flashed 319MB KK)
BuildID: 20150413010203
Gaia: 3c68964cb9fdba7cf0f6829b7f44562acaf1f1d7
Gecko: 0a46652bd992
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Device: Flame 2.2 (full flashed 319MB KK)
BuildID: 20150413002502
Gaia: cec00d643f517ffd96cde559cd3bbd43ab85816c
Gecko: 5005522fd68e
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

------

This issue does NOT reproduce on Flame 2.1. Kinetic scrolling effect is shown when scrolling horizontally.

Device: Flame 2.1 (full flashed 319MB KK)
BuildID: 20150413001204
Gaia: bbe983b4e8bebfec26b3726b79568a22d667223c
Gecko: a1b2434ad001
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 2

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

Thanks! A regression-window on 2.2 would also be good then.

Comment 3

[Botond Ballo [:botond]]

I think this was regressed by bug 1066888. Dropping regressionwindow-wanted flag while I confirm and investigate.

Comment 4

[Botond Ballo [:botond]]

Specifically, it was regressed by the addition of this block [1]. As a result of that code, we are dropping the velocity along the x-axis because the scrollgrabbing container can't scroll horizontally, even though an APZC higher up in the handoff chain can.

I don't immediately recall the motivation for adding this code, nor is it documented in bug 1066888; I'll think about it a bit.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?rev=86b60ef17965#444

Comment 5

[Brogan Zumwalt [Inactive]]

From the Central Regression window (below), bug 1066888 is listed in the pushlog so that lends some credence to your findings Botond. Just saw your comments so I will hold off on finding mozilla-inbound window for now.


Last working Central build:
Device: Flame 2.2
BuildID: 20141031125656
Gaia: 5964f1339f37e7595aff7de7512b8529bc640b76
Gecko: a264cdd47217
Version: 36.0a1 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First broken Central build:
Device: Flame 2.2
Build ID: 20141031131456
Gaia: 5964f1339f37e7595aff7de7512b8529bc640b76
Gecko: 12ac66e2c016
Version: 36.0a1 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0


Working Gaia with Broken Gecko issue DOES reproduce:
Gaia: 5964f1339f37e7595aff7de7512b8529bc640b76
Gecko: 12ac66e2c016

Working Gecko with Broken Gaia issue does NOT reproduce:
Gaia: 5964f1339f37e7595aff7de7512b8529bc640b76
Gecko: a264cdd47217


Central pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a264cdd47217&tochange=12ac66e2c016

Comment 6

[Botond Ballo [:botond]]

I recall the motivation now: suppose you have a child scroll frame C, and parent scroll frame P, and they can both only scroll vertically. If you do a diagonal fling on C, then without the block added in bug 1066888, the horizontal component of the fling would cause the fling to go into overscroll and be handed off to P, even though P can't scroll horizontally either, and C hasn't reached the end of its vertical scroll range.

With this added block, we truncate components of the velocity that are in a direction that the flinging APZC doesn't have room to scroll in, and only do handoff if there is still overscroll. However, as we've discovered in this bug, the truncation can be undesirable in cases where a parent scroll frame could have scrolled in the truncated direction.

I think the proper way to handle a situation where we reach the end of the scroll range in one direction but not the other, is to do a partial handoff, i.e. only hand off the portion of the fling in the one direction.

This is a bit of a larger change, though, so for the purposes of fixing this 2.2 regression, I propose working around the problem by restricting the truncation to the case where a parent scrollable doesn't have room to scroll along the axis in question, either.

Comment 7

[Botond Ballo [:botond]]

Attachment: MozReview Request: bz://1153711/botond

/r/7027 - Bug 1153711 - Do not discard a component of a fling if an APZC further in the handoff chain has room to scroll in that direction. r=kats

Pull down this commit:

hg pull -r f14d3c29a1ad36e1481c1eff71ab96a51a312f95 https://reviewboard-hg.mozilla.org/gecko/

Comment 8

[Botond Ballo [:botond]]

Try push that includes this patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e7237a0715f

Comment 9

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

Comment on attachment 8592268 [details]
MozReview Request: bz://1153711/botond

https://reviewboard.mozilla.org/r/7025/#review5853

::: gfx/layers/apz/src/AsyncPanZoomController.cpp
(Diff revision 1)
> +  default:                MOZ_ASSERT(false); return false;

One day we'll be able to scroll in the z-direction!

Comment 10

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

Comment on attachment 8592268 [details]
MozReview Request: bz://1153711/botond

https://reviewboard.mozilla.org/r/7025/#review5855

Ship It!

Comment 12

[Botond Ballo [:botond]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a365d588c8df

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Please request b2g37 approval on this patch when you get a chance.

Comment 15

[Botond Ballo [:botond]]

Comment on attachment 8592268 [details]
MozReview Request: bz://1153711/botond

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 #): 
  Bug 1066888 (physics tweaks for overscroll effect).

User impact if declined: 
  When releasing the finger after a horizontal pan in
  the B2G browser, momentum scrolling does not happen.

Testing completed: 
  Tested locally, and on m-c since 2015-04-15.

Risk to taking this patch (and alternatives if risky): 
  Low - this is small, targeted fix.

String or UUID changes made by this patch:
  None.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for b2g37 uplift.

Comment 17

[Botond Ballo [:botond]]

Attachment: Fix, rebased to apply to b2g37 branch

Rebased patch to apply to b2g37 branch.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/a25acc174d84

Comment 19

[Brogan Zumwalt [Inactive]]

Issue verified fixed on Flame 2.2 and 3.0

Flinging horizontally on test page continues scrolling without further user input until deceleration or if interrupted by user. Vertical flinging works in same way.

Device: Flame 2.2
Build ID: 20150428002500
Gaia: 9f6b1b9082662ba2c14168fc66bb02b4df3141e5
Gecko: e79c19bf19bf
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Device: Flame 3.0
Build ID: 20150428010206
Gaia: 0636405f0844bf32451a375b2d61a2b16fe33348
Gecko: caf25344f73e
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Comment 21

[Botond Ballo [:botond]]

Attachment: MozReview Request: Bug 1153711 - Do not discard a component of a fling if an APZC further in the handoff chain has room to scroll in that direction. r=kats