Closed Bug 1349828 Opened 7 years ago Closed 7 years ago

Animation of scrolling tab strip finishes too sharp when scrolling to the beginning/end of tab stip

Categories

(Toolkit :: UI Widgets, defect)

53 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla55
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- wontfix
firefox54 --- verified
firefox55 --- verified

People

(Reporter: 684sigma, Assigned: masayuki)

References

Details

(Keywords: regression)

Attachments

(1 file)

I have a problem with Firefox Beta 53. It doesn't happen in Firefox ESR 45
Sometimes animation of scrolling in tab strip finishes too sharp.
I noticed one specific scenario when it happens

1. Open many tabs so that some tabs are invisible, scroll tab strip to the beginning
2. Hover mouse over tab strip and quickly rotate mouse wheel down several times to scroll tab strip to the end

Result: Tab strip scrolls the the end too sharp
Expected: Animation should slow down in the end, like it happens when I triple-click on ">" button in tab strip
Has STR: --- → yes
Keywords: regression
Build ID: 20170323030203
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0

Reproducible on Nightly 55.0a1, 54.0a1 and 53.0a1 on Windows 10 x 64.
Status: UNCONFIRMED → NEW
Ever confirmed: true
The regression was made in two steps.

First step: normal smooth scrolling was changed to scrolling without animation
Mozregression-gui generated this regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=75174a724d6516bb643710498862a44feeedde6d&tochange=f4f6a59b53106e9ff550b18e374a401ecea78e2a
->
1316505 – Tabbar scrolls too fast with wheel supporting high resolution scroll on Windows
https://bugzilla.mozilla.org/show_bug.cgi?id=1316505

Second step: animation was added again, but it is now not working correctly (see Comment #0)
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4d1a2b1de7f53acaa8cbd41a727efd4637e788a7&tochange=9fc13b45970ab17567ff53ff4f34a6a605852707
->
1320609 – tabbar should scroll smoothly even when wheel device doesn't support high resolution scroll
https://bugzilla.mozilla.org/show_bug.cgi?id=1320609
Blocks: 1320609, 1316505
Has Regression Range: --- → yes
Component: Tabbed Browser → XUL Widgets
Flags: needinfo?(masayuki)
Product: Firefox → Toolkit
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Flags: needinfo?(masayuki)
Comment on attachment 8856964 [details]
Bug 1349828 Smooth scroller of <scrollbox> should not try to scroll to outside of the range

https://reviewboard.mozilla.org/r/128760/#review132008
Attachment #8856964 - Flags: review?(mstange) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/b01f44c896b2
Smooth scroller of <scrollbox> should not try to scroll to outside of the range r=mstange
https://hg.mozilla.org/mozilla-central/rev/b01f44c896b2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Please request Aurora approval on this when you're comfortable doing so.
Flags: needinfo?(masayuki)
Reporter, could you check if this bug is actually fixed on Nightly?
https://www.mozilla.org/en-US/firefox/channel/desktop/
Flags: needinfo?(684sigma)
User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0
Nightly 55 (2017-04-14)

This bug is fixed
Flags: needinfo?(684sigma)
Comment on attachment 8856964 [details]
Bug 1349828 Smooth scroller of <scrollbox> should not try to scroll to outside of the range

Thank you for your report and confirmation of the fix!

Approval Request Comment
[Feature/Bug causing the regression]:
For Windows and Linux users, looks like this is a regression of bug 1316505 and bug 1320609. For most (not using 3rd party's mouse) Mac users, this is a long standing bug.

[User impact if declined]:
Scroll tab bar, XUL scrollable elements with mouse wheel (or swipe of touchpad) may cause too fast scroll even when it reaches the end soon. (No problem for operation, just a bug of look and feel.)

[Is this code covered by automated tests?]:
No because it's impossible to test the scroll speed down when scrolled area will reach its end soon.

[Has the fix been verified in Nightly?]:
Yes.

[Needs manual test from QE? If yes, steps to reproduce]:
Yes.

1. Open a lot of tabs (it's better to open different favicon pages for easier to check scroll speed by your eyes) to scroll tabs (easier to test this with narrower window).
2. Turn mouse wheel much fast on tab bar to scroll tabs to the end.

Then, scroll speed should be slower down when it reaches the end of its left-most or right-most tab.

If you use some pointing devices which support high resolution scrolling, e.g., trackpad of MacBook, expensive mice of Logitech, it may be difficult to see the difference of the behavior between 53 and Nightly.

[List of other uplifts needed for the feature/fix]:
No.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
The cause of this bug is, <scrollbox>'s smooth scroller stores scroll destination everywhere. I.e., it may store scroll destination as outside of actual scrollable range.  This patch clamps the destination into scrollable range.  Therefore, the scroll speed becomes slow down when it reaches the end soon. (I.e., this patch doesn't touch complicated animation logic.)

[String changes made/needed]:
No.
Flags: needinfo?(masayuki)
Attachment #8856964 - Flags: approval-mozilla-aurora?
Comment on attachment 8856964 [details]
Bug 1349828 Smooth scroller of <scrollbox> should not try to scroll to outside of the range

Fix an animation of scrolling tab issue and was verified. Aurora54+.
Attachment #8856964 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
Verified as fixed on Firefox 54 beta 4 and latest Nightly 55.0a1 under Win 10 64-bit, Mac OS X 10.11 and Ubuntu 16.04 64-bit.
The issue was most visible on Windows.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: