buggy scrolling with keyboard at twitter.com

VERIFIED FIXED in Firefox 48

Status

()

defect
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: euthanasia_waltz, Assigned: kats)

Tracking

({regression})

48 Branch
mozilla48
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox47 unaffected, firefox48- verified)

Details

(Whiteboard: gfx-noted)

Attachments

(2 attachments)

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
Blocks: 1253860
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(bugmail.mozilla)
Keywords: regression
[Tracking Requested - why for this release]:
Version: Trunk → 48 Branch
(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!
Assignee: nobody → bugmail.mozilla
Flags: needinfo?(bugmail.mozilla)
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?
This fixes it, but I'd like to wait and see what Markus' patch looks like.
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 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.
Attachment #8729581 - Flags: review?(botond) → review+
Comment on attachment 8729582 [details] [diff] [review]
Part 2 - Generalize the NotifyLayersUpdated short-circuit

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

Nice!
Attachment #8729582 - Flags: review?(botond) → review+
(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!
https://hg.mozilla.org/mozilla-central/rev/20c27df0407c
https://hg.mozilla.org/mozilla-central/rev/f0802b8a0863
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Flags: qe-verify+
QA Contact: petruta.rasa
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!
Flags: needinfo?(bugmail.mozilla)
(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!
Flags: needinfo?(bugmail.mozilla)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.