1255705 - buggy scrolling with keyboard at twitter.com

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect)

Description

[atlanto]

STR
1. Open twitter page (e.g. https://twitter.com/firefox)
2. Hit down arrow key several times, and hit up arrow key several times
3. Repeat step.2

ER
page scrolls without problem
AR
page doesn't scroll normally

mozregression says
>INFO: Looks like the following bug has the changes which introduced the regression:
>https://bugzilla.mozilla.org/show_bug.cgi?id=1253860

Comment 1

[Alice0775 White]

[Tracking Requested - why for this release]:

Comment 2

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

(In reply to atlanto from comment #0)
> page doesn't scroll normally

I see a couple of issues - one is the header background image running away/jumping, which is filed as bug 1255068.

The other is that on some keyboard scrolls the page seems to move and then jump back to where it was. Seems to happen around where the top background image is half scrolled off. If I continue scrolling I'm able to get past it into the bottom part of the page which works fine. I'll use this bug to track that issue.

If what you're seeing is something different please describe it in more detail, thanks!

Comment 3

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

I looked into this. It's basically the same problem I described at https://bugzilla.mozilla.org/show_bug.cgi?id=1253860#c7 but for empty transactions, instead of transactions on a different layer tree. The empty transaction comes in with a stale metrics, and clobbers the scroll offset on the APZ side. The guard I added to fix the other problem doesn't work here because aThisLayerTreeUpdated is true for the empty transaction case.

I think markus' fix for bug 1255068 will probably fix this, but we can probably fix it in a different way as well. Twice the fix, twice the better?

Comment 4

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

Attachment: Part 1 - Logging

Comment 5

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

Attachment: Part 2 - Generalize the NotifyLayersUpdated short-circuit

This fixes it, but I'd like to wait and see what Markus' patch looks like.

Comment 6

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

After looking at Markus' patch I think it's still worth landing these patches. His patches seem specific to empty transactions that carry transform changes, but according to our IRC discussion there might be other kinds of empty transactions as well, like opacity changes and such. If those come in interleaved with NotifyScrollUpdated messages then we'd have the same problem. These patches should deal with all the kinds of empty transactions.

Comment 7

[Botond Ballo [:botond]]

Comment on attachment 8729581 [details] [diff] [review]
Part 1 - Logging

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

::: layout/generic/nsGfxScrollFrame.cpp
@@ +69,5 @@
>  #include "mozilla/unused.h"
>  #include <algorithm>
>  #include <cstdlib> // for std::abs(int/long)
>  #include <cmath> // for std::abs(float/double)
> +#include "LayersLogging.h"  // for Stringify

I'm not very picky about #include ordering and this file is a mess to begin with, but let's at least not interleave standard library includes and other includes.

Comment 8

[Botond Ballo [:botond]]

Comment on attachment 8729582 [details] [diff] [review]
Part 2 - Generalize the NotifyLayersUpdated short-circuit

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

Nice!

Comment 9

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

(In reply to Botond Ballo [:botond] from comment #7)
> I'm not very picky about #include ordering and this file is a mess to begin
> with, but let's at least not interleave standard library includes and other
> includes.

There's already <stdint.h> somewhere in the middle of the pile :) But yeah I moved my new #include up a few lines. Thanks for the quick reviews!

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/20c27df0407c
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0802b8a0863

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/20c27df0407c
https://hg.mozilla.org/mozilla-central/rev/f0802b8a0863

Comment 12

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this bug using Fx48.0a1 build(20160310030242).
This bug no longer occurs using Fx48.0b2 build(20160620091522) on Windows 10x64, Ubuntu 10.04 x86 and Mac OS X10.10; 

Notes:
-If the user narrow the width of the window; the left-right scroll it not functioning with the left-right arrows keys.
-There still is a smaller glitch when reaching the Firefox's logo when scrolling up: http://imgur.com/2qM9Nxv

Please review and let me know if I should file these issues as follow ups. Thank you!

Comment 13

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

(In reply to Cristian Comorasu from comment #12)
> I reproduced this bug using Fx48.0a1 build(20160310030242).
> This bug no longer occurs using Fx48.0b2 build(20160620091522) on Windows
> 10x64, Ubuntu 10.04 x86 and Mac OS X10.10; 

Thanks!

> Notes:
> -If the user narrow the width of the window; the left-right scroll it not
> functioning with the left-right arrows keys.

This appears to be the same behaviour with and without e10s, and the same in Chrome as well. I think it's not a bug.

> -There still is a smaller glitch when reaching the Firefox's logo when
> scrolling up: http://imgur.com/2qM9Nxv

This looks like bug 1103394. Can you please file a new bug for this and find a regression range if there is one? Thanks!

Comment 14

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #13)
> (In reply to Cristian Comorasu from comment #12)
> > I reproduced this bug using Fx48.0a1 build(20160310030242).
> > This bug no longer occurs using Fx48.0b2 build(20160620091522) on Windows
> > 10x64, Ubuntu 10.04 x86 and Mac OS X10.10; 
> 
> Thanks!

Marking as Verified Fixed.
 
> > -There still is a smaller glitch when reaching the Firefox's logo when
> > scrolling up: http://imgur.com/2qM9Nxv
> 
> This looks like bug 1103394. Can you please file a new bug for this and find
> a regression range if there is one? Thanks!

Filed bug 1281764 , the issue reproduces since Fx10, I think there is no regression.