1232048 - Home Screen can be stopped between panels if panning finger is moving slowly or is still when it is released.

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect)

Description

[Martin Shuman [:Marty]]

Attachment: logcat_Homescreen-pan.txt

Description:
If the user is moving between the panels slowly, or has stopped panning entirely halfway when they release their finger, the Homescreen will stop between the two panels.

Note: This issue appears to be a recent regression of bug 1219391, which was resolved fixed on 10/29/2015.

Repro Steps:
1) Update a Aries to 20151211115227
2) At the homescreen, pan halfway between the two homescreen panels.
3) Keep your finger still between the panels, then release your finger from the screen

Actual:
The homescreen stops and rests between the two panels.

Expected:
The Homescreen snaps completely to the nearest panel.

Environmental Variables:
Device: Aries 2.6
Build ID: 20151211115227
Gaia: d886e152b42e557db612470d3417fe2947cbdeee
Gecko: 754b4805a65cab4f3aca99899227acc44ba4fb20
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 45.0a1 (2.6)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Repro frequency: 8/10
See attached: Video (URL), Logcat

Comment 1

[Martin Shuman [:Marty]]

This issue DOES occur on the latest Flame 2.6 build.
The homescreen stops and rests between the two panels.

Environmental Variables:
Device: Flame 2.6
BuildID: 20151211030233
Gaia: d886e152b42e557db612470d3417fe2947cbdeee
Gecko: 754b4805a65cab4f3aca99899227acc44ba4fb20
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 45.0a1 (2.6) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

*****************************************

This issue does NOT occur on the latest Flame Nightly 2.5 build, yesterday's Flame Nightly 2.6 build, or the 12/09 Aries Dogfood build.
The Homescreen snaps completely to the nearest panel.

Environmental Variables:
Device: Flame 2.5
BuildID: 20151211164817
Gaia: 062e0fa48c1b5d5156f6911a5a4519128fac3231
Gecko: ded0306e0d78528cdcb237c8b05543390f10ec96
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 44.0a2 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0

Environmental Variables:
Device: Aries 2.6
BuildID: 20151209121803
Gaia: 961528f4391668bc89ec0be14fa367cea099b588
Gecko: 319be5e7ce3061c7c16f24d750b6dacdbcac4c35
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 45.0a1 (2.6) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Environmental Variables:
Device: Flame 2.6
BuildID: 20151210030222
Gaia: 961528f4391668bc89ec0be14fa367cea099b588
Gecko: b40ba117fa757861c9caa660ae989122718b494b
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 45.0a1 (2.6) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Comment 2

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Nominating to block based on bug 1219391 also being a blocker.

Let's get a window here.

Comment 3

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

mozilla-inbound regression window:

Last Working
Device: Flame 2.6
BuildID: 20151209070520
Gaia: 961528f4391668bc89ec0be14fa367cea099b588
Gecko: df9842398da30e4a0ee5549be0d3fd75ab73e032
Version: 45.0a1 (2.6) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

First Broken
Device: Flame 2.6
BuildID: 20151209071542
Gaia: 961528f4391668bc89ec0be14fa367cea099b588
Gecko: d55129069a1f56f63573282911ad2dac17d6e7b1
Version: 45.0a1 (2.6) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Gaia is the same so it's a Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=df9842398da30e4a0ee5549be0d3fd75ab73e032&tochange=d55129069a1f56f63573282911ad2dac17d6e7b1

Caused by changes made in Bug 1228407.

Comment 4

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Kartikaya the changes made for bug 1228407 seem to have caused this issue.  Can you please take a look?

Comment 5

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

Thanks, I'll look into it.

Comment 6

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

So in bug 1228407 (part 1) I added this comment:

            // Note that if we get two smooth-scroll requests to the same
            // destination with some other scroll in between, mDestination will
            // get reset and so we shouldn't have the problem where this check
            // discards a legitimate smooth-scroll.

What I overlooked here is that mDestination can get set to something, and then reset back to the original thing, thereby discarding a legitimate smooth-scroll. Specifically what's happening here is that:
- layout does a snap to (0,0) and propagates it to APZ
- user pans on the homescreen. APZ sends repaint requests to layout, which updates mDestination
- user lifts finger. APZ sends snap request to layout, which snaps to 0,0 [1] and puts that in mDestination a few lines below. Then it goes into the smooth-scroll code below that and takes the short-circuit path and returns without sending the snap request.

[1] https://dxr.mozilla.org/mozilla-central/rev/cb66ffeb6725e8344818e8e2f707ae2eaeb953b4/layout/generic/nsGfxScrollFrame.cpp#2086

Comment 7

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

Attachment: Patch

This fixes it, but I need to check to make sure it doesn't regress bug 1228407.

Comment 8

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

Attachment: Patch

Verified that it doesn't regress. Updated a comment as well.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/83d4ac013262

Comment 10

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

Comment on attachment 8698677 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: bug 1228407
[User impact if declined]: scroll snapping may not work properly in some cases. In particular, when two snaps happen to the same scroll offset with a non-smooth scroll in between. This only manifests if APZ is enabled so right now it doesn't affect 45 but I'd like to uplift it anyway to go with the regressing bug.
[Describe test coverage new/current, TreeHerder]: tested manually
[Risks and why]: low risk, pretty straightforward change
[String/UUID change made/needed]: none

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8698677 [details] [diff] [review]
Patch

Sure, taking it. Some people might try APZ.

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4951523aa064

Comment 13

[John Thomas [:Johnt]]

This issue is verified fixed for Aries 2.6
Result: Unable to stop between Homescreen panels.

Environmental Variables:
Device: Aries 2.6 [Full Flash]
BuildID: 20160224110426
Gaia: 4f0e2a1a42a2d049b6fe8f4f095cdcdf0fd5465c
Gecko: d848a5628d801a460a7244cbcdea22d328d8b310
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 47.0a1 (2.6) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:47.0) Gecko/47.0 Firefox/47.0

Comment 14

[John Thomas [:Johnt]]

This issue is also Verified fixed for Flame 2.6.
Result: Unable to stop between Homescreen panels.

Environmental Variables:
Device: Flame KK 2.6 [Full Flash](512)
BuildID: 20160225030411
Gaia: 4f0e2a1a42a2d049b6fe8f4f095cdcdf0fd5465c
Gecko: c1e0d1890cfee9d86c8d566b0490053f21e0afc6
Gonk: 8a066f7fa7410e32b58def35f322aa33f03db283
Version: 47.0a1 (2.6) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:47.0) Gecko/47.0 Firefox/47.0