1607181 - Tab strip does not scroll smoothly with mousewheel until first clicking on the scroll button

Status: VERIFIED FIXED

(Toolkit :: UI Widgets, defect, P3)

Description

[Kestrel]

STR:

1. Click the new tab button 20 times or enough to trigger overflow.
2. Scroll tab strip with mousewheel.

Expected:

Tab strip scrolls smoothly.

Actual:

Tab strip scrolls without animation.

Can be corrected by clicking overflow arrow button to perform a smooth scroll.

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=94eaa02e0ea1725512893cd5139b1d200fcf4fdd&tochange=5e221240fc1d537da612973800bbe24efa50736b

Regressed by Bug 1514926.

Comment 1

[Brian Grinstead [:bgrins]]

I believe this is a duplicate of Bug 1601184 (fixed in version 73), if this is the same functionality that requires the toolkit.tabbox.switchByScrolling pref to be set.

Comment 2

[Kestrel]

It's reproducible in Nightly 74.0a1 (2020-01-06) with a new profile and is unrelated to tab switching with the mousewheel.

Comment 3

[Brian Grinstead [:bgrins]]

(In reply to Kestrel from comment #2)

It's reproducible in Nightly 74.0a1 (2020-01-06) with a new profile and is unrelated to tab switching with the mousewheel.

I see, you are correct - I can reproduce this on Windows 7. It works as expected (smooth scroll without clicking the button first) on OSX.

Comment 4

[Brian Grinstead [:bgrins]]

OK, I see that for some reason [smoothscroll=true] isn't getting set on tabbrowser-arrowscrollbox until after clicking on the scroll button. I'm not sure if it's intentional - the same thing happens on OSX but the scroll is smooth anyway.

Comment 5

[Brian Grinstead [:bgrins]]

The XBL implementation set the attribute in the constructor if it wasn't already: https://hg.mozilla.org/mozilla-central/file/94eaa02e0ea1725512893cd5139b1d200fcf4fdd/toolkit/content/widgets/scrollbox.xml#l34. This check has been moved into the smoothScroll getter instead, which isn't called in connectedCallback. I think this was just an oversight in the conversion.

Comment 6

[Brian Grinstead [:bgrins]]

Attachment: Bug 1607181 - eagerly set [smoothscroll] on arrowscrollbox instead of waiting for the getter to be called;r=dao

The XBL implementation set the attribute in the constructor if it wasn't already:

• https://hg.mozilla.org/mozilla-central/file/94eaa02e0ea1725512893cd5139b1d200fcf4fdd/toolkit/content/widgets/scrollbox.xml#l34.

In the Custom Element conversion this check was moved into the getter, which isn't called until a scrollbutton is clicked,
which meant that we'd get non-smooth scrolling if just mousewheeling the button. I can't find any reason for intentionally
changing that in the bug or review, so I think it was just an oversight.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80ca2534a3e2
eagerly set [smoothscroll] on arrowscrollbox instead of waiting for the getter to be called;r=dao

Comment 8

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/80ca2534a3e2

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance.

Comment 10

[Alin Ilea, Desktop Test Engineering]

Reproduced the initial issue using Beta build 73.0b2 (Build id: 20200107212705).
Verified - Fixed in latest Nightly build 74.0a1 (Build id: 20200108215606) using Windows 10.

Comment 11

[Brian Grinstead [:bgrins]]

Comment on attachment 9119177 [details]
Bug 1607181 - eagerly set [smoothscroll] on arrowscrollbox instead of waiting for the getter to be called;r=dao

Beta/Release Uplift Approval Request

• User impact if declined: Tab scrolling when using the mouse wheel on tab arrow buttons doesn't work until first clicking on the arrow buttons
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a small change (setting an attribute on the tab scrollbox earlier), and it's restoring the logic to more closely match the old XBL implementation. Fix has been verified on Nightly, and no regressions have been reported in the week or so it's been there.
• String changes made/needed:

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9119177 [details]
Bug 1607181 - eagerly set [smoothscroll] on arrowscrollbox instead of waiting for the getter to be called;r=dao

Fixes a regression with tab strip scrolling with the mouse wheel. Approved for 73.0b5.

Comment 13

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/087b0cab93cc

Comment 14

[Alin Ilea, Desktop Test Engineering]

Verified - Fixed in latest Beta build 73.0b5 (Build id: 20200115020958) using Windows 10.