If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Scrolling using arrow keys causes weird element positioning behavior

RESOLVED FIXED in mozilla23

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Alice0775 White, Assigned: karlt)

Tracking

({regression})

15 Branch
mozilla23
x86
All
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/e21173ed2c38
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0 ID:20120623053647

This is spin off Bug 756400 
After landing Bug 763838

STR:
1. Open the following URL: https://bugzilla.mozilla.org/describecomponents.cgi
2. Scroll down by hold down arrow key so the very bottom of the page is 'touched'

Actual results:
Right after page is reached the bottom edge, "Mozilla Services:"and Icon  moves to upward in a pixel.


Expected Results:
 Should not move.
(Reporter)

Comment 1

5 years ago
Screen capture
http://space.geocities.jp/alice0775/test/20120625_1213_45.swf.html
(Assignee)

Comment 2

5 years ago
Visible on Linux (mouse wheel or keys) as well as Win 7.
Try zooming in/out 1 level if fonts mean it doesn't reproduce at default size.  Regression window for me includes bug 681192.

I suspect this is the same issue as bug 756400 comment 8.
Blocks: 681192
Keywords: regression
OS: Windows 7 → All
(Reporter)

Comment 3

5 years ago
STR:
1. Open the following URL: https://bugzilla.mozilla.org/describecomponents.cgi
2. Zoom in 1 or 2 level
3. Click empty area to remove focys from input element
4. Scroll down by hold down arrow key so the very bottom of the page is 'touched'

Actual results:
Right after page is reached the bottom edge, test such as "Mozilla Localizations:Translation, spelling and other errors in language packs and localized builds (more info)"  moves to upward in a pixel.

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/6fe7dd2f8f57
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120510021321
Bad:
http://hg.mozilla.org/mozilla-central/rev/b7b6565d12a0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120510050721
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6fe7dd2f8f57&tochange=b7b6565d12a0


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/b5304fd23df9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120509222721
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/67091352b7d2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120509223521
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b5304fd23df9&tochange=67091352b7d2



Last good: c3d3bfb3b68d
First bad: 9d9a3edaa0b9

Triggered by
9d9a3edaa0b9	Robert O'Callahan — Bug 681192. Part 11: Don't snap scrollrange endpoints to device pixels anymore. r=matspal
Version: Trunk → 15 Branch
(Assignee)

Updated

5 years ago
QA Contact: karlt
(Assignee)

Updated

5 years ago
Assignee: nobody → karlt
QA Contact: karlt
(Assignee)

Updated

5 years ago
Blocks: 810274
(Assignee)

Comment 4

5 years ago
There is an asymptotic slow-down when reaching the end of the scroll range if the key is still down.  This means there are half a dozen sub-pixel scrolls and the final sub-pixel scroll does not happen until the mouse is released.

Besides the effects reported here and in bug 756400 comment 10, it also makes me want to push the key harder because things are slowing down so much.  Perhaps this is the slowing down too much referred to in bug 206438 comment 69, but I haven't seen the "aiming past the edge" code.
(Assignee)

Comment 5

5 years ago
Created attachment 736703 [details] [diff] [review]
Part 1: move current motion calculation into InitSmoothScroll

I'm going to use mIsFirstIteration in Part 3.  The rest of the rearranging
here is not necessary but moving the work on the AsyncScroll into the class and reducing the number of parameters that need to be passed.  Perhaps its a little odd to pass the start pos to the constructor when it is not used by non-smooth scroll, but it fits in with the way it is used here.
Attachment #736703 - Flags: review?(roc)
(Assignee)

Comment 6

5 years ago
Created attachment 736704 [details] [diff] [review]
Part 2: refactor AsyncScroll duration API so duration can be evaluated before resetting

This is for part 3.
Attachment #736704 - Flags: review?(roc)
(Assignee)

Comment 7

5 years ago
Created attachment 736705 [details] [diff] [review]
Part 3: don't let additional events with the same destination restart smooth scrolling

Markus, do you have time to give this a quick look, please?
Even if you can just think about the idea, then please just mark the feedback flag and I can find another reviewer.

Each time an event is received the duration is reset, and minimum duration
means that the finish time is pushed further and further into the future.  If
the destination is the same, then the only reason for recalculating the
animation would be if the duration is reduced because events are being
received more frequently, indicating a desire to scroll faster.
Attachment #736705 - Flags: review?(mstange)
Attachment #736705 - Flags: feedback?(mstange)
(Assignee)

Comment 8

5 years ago
Part 3 deals with most of the problem but there can still be multiple subpixel scrolls when scrolling small distances with long durations (e.g. with the mouse wheel).

My follow-up patches to deal with that are currently having trouble with toolkit/content/tests/chrome/test_tooltip.xul on MacOS 10.8
https://tbpl.mozilla.org/?tree=Try&rev=eb07014b7c0c

Comment 9

5 years ago
Just dropping by to say this problem can be very noticeable while entering full screen.

STR:
1. Open https://bugzilla.mozilla.org/describecomponents.cgi
2. Enter full screen mode
3. Watch it dance!
Attachment #736703 - Flags: review?(roc) → review+
Attachment #736704 - Flags: review?(roc) → review+
(Assignee)

Comment 10

5 years ago
(In reply to Bram Speeckaert from comment #9)
> STR:
> 1. Open https://bugzilla.mozilla.org/describecomponents.cgi
> 2. Enter full screen mode
> 3. Watch it dance!

Looks like the animation to slide the browser chrome off the top of the screen and slide the content up is not using layer pixel aligned frames.  The patches here are not going to deal with that unfortunately.

Comment 11

5 years ago
So should I be filing a separate bug for that then?
Comment on attachment 736705 [details] [diff] [review]
Part 3: don't let additional events with the same destination restart smooth scrolling

Good idea. This fixes the extreme slowdown without sacrificing the smoothness of the animation.
Attachment #736705 - Flags: review?(mstange)
Attachment #736705 - Flags: review+
Attachment #736705 - Flags: feedback?(mstange)
(Assignee)

Comment 13

5 years ago
(In reply to Bram Speeckaert from comment #11)
> So should I be filing a separate bug for that then?

Yes, please.
(Assignee)

Comment 14

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f687fc38de90
https://hg.mozilla.org/integration/mozilla-inbound/rev/dae9cddd40cb
https://hg.mozilla.org/integration/mozilla-inbound/rev/0795073314a9

I'll deal with the remaining causes of multiple subpixel scrolls in bug 810274.

Comment 15

5 years ago
(In reply to Karl Tomlinson (:karlt) from comment #13)
> (In reply to Bram Speeckaert from comment #11)
> > So should I be filing a separate bug for that then?
> 
> Yes, please.

Filed bug 863170.
https://hg.mozilla.org/mozilla-central/rev/f687fc38de90
https://hg.mozilla.org/mozilla-central/rev/dae9cddd40cb
https://hg.mozilla.org/mozilla-central/rev/0795073314a9
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.