Closed
Bug 1387130
Opened 7 years ago
Closed 7 years ago
Animation of scrolling tabstrip with scrollbutton is not smooth. It scrolls as the inchworm
Categories
(Toolkit :: UI Widgets, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | unaffected |
firefox57 | --- | verified |
People
(Reporter: alice0775, Assigned: mconley)
References
Details
(Keywords: regression, Whiteboard: [photon-performance][qa-commented])
Attachments
(1 file, 1 obsolete file)
Reproducible: always Steps To Reproduce: 1. Open many 20-30 tabs 2. Scroll tabstrip with scrollbutton Actual Results: It scrolls one by one as the inchworm Expected Results: smooth
Updated•7 years ago
|
Flags: needinfo?(dao+bmo)
Comment 2•7 years ago
|
||
More information from bug 1387763 (resolved dup of bug 1388070 , further resolved dup of this bug. ) 57.0a1 (2017-08-06) (32-bit) continues the jerky & slow behaviour with < or > tab-bar scrolling Using mouse-wheel to scroll tabs remains acceptable smooth scroll. [ Keyboard arrows have no effect when tab is selected. I'm unsure of design spec. ] + Sometimes if I hold the "<", the tabs will move, but when I release the "<", they jump a further few places Playing around, I did note that using < or > can cause quite erratic jumps, worse under conditions of load = many browser windows & many tabs (+ maybe youtubes as well). Running under win10, on an oldish laptop Dell D520 (adequate, 4GB, Core2 @ 1.83GHz) Works ok if I am cautious & careful). I suspect less of a problem for more capable rigs. + I think there was a config item at one time to smooth-scroll tab-bar (which still happens if I use mousewheel); I wonder if that is/was relevant. + Noted also, possibly irrelevant clue: horizontal scrolling inside a large rendered webpage seems to have similar behaviour, moves in jumps.
Comment 3•7 years ago
|
||
Also as :Gijs noted on 7Aug17: "There's a regression range in bug 1388070, so duping there." Q.v.
Comment 5•7 years ago
|
||
Adding to Photon team for triage.
Flags: needinfo?(mmucci)
Whiteboard: [photon-performance] [triage]
Updated•7 years ago
|
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Flags: needinfo?(dao+bmo)
Updated•7 years ago
|
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-performance] [triage] → [reserve-photon-performance]
Updated•7 years ago
|
Flags: qe-verify? → qe-verify+
Updated•7 years ago
|
QA Contact: adrian.florinescu
Workaround userChrome.css that restores functionality at the expense of smooth scrolling:
> .tabbrowser-arrowscrollbox > scrollbox {
> scroll-behavior: unset;
> }
Comment 9•7 years ago
|
||
Agreed, similar to bug 1387718 Dragging a tab to place it beyond the < >, movement is slower than a pulmonate gastropod.
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(mconley)
Assignee | ||
Comment 10•7 years ago
|
||
Stealing with dao's permission. From dao in IRC: "I think we basically just need to bring back _arrowScrollAnim. at least that would be the simplest way to undo the regression"
Assignee: dao+bmo → mconley
Flags: needinfo?(mconley)
Updated•7 years ago
|
Iteration: --- → 57.2 - Aug 29
Whiteboard: [reserve-photon-performance] → [photon-performance]
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•7 years ago
|
||
Comment on attachment 8901371 [details] Bug 1387130 - Increase the speed with which the scrollarrow scroll a scrollbox. Is this too naive a fix? This certainly feels smoother to me, and it's simple as hell, but I suspect I'm missing some nuance.
Attachment #8901371 -
Flags: feedback?(dao+bmo)
Comment 13•7 years ago
|
||
Comment on attachment 8901371 [details] Bug 1387130 - Increase the speed with which the scrollarrow scroll a scrollbox. This is faster for sure, but also makes it a bit harder to keep track of the scrolling. I think a bit slower but continuous scroll animation would be preferable.
Attachment #8901371 -
Flags: feedback?(dao+bmo)
Updated•7 years ago
|
Iteration: 57.2 - Aug 29 → 57.3 - Sep 19
Assignee | ||
Comment 14•7 years ago
|
||
Hey dao, when you said "bring back _arrowScrollAnim" (quoted you in IRC in comment 10), I assume you didn't mean essentially backing out bug 1356705, right? Did you mean that we have a state object that knows which index we're supposed to be scrolling to, which updates off of a timer?
Flags: needinfo?(dao+bmo)
Comment 15•7 years ago
|
||
I meant specifically this object and the code using it: https://hg.mozilla.org/mozilla-central/rev/3845af01ea9f#l2.373
Flags: needinfo?(dao+bmo)
Assignee | ||
Comment 16•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #15) > I meant specifically this object and the code using it: > > https://hg.mozilla.org/mozilla-central/rev/3845af01ea9f#l2.373 Right - but that object is powering itself using rAF and setting scrollPosition, which would re-introduce sync flushes. Do you mean to put the same object back, but power it using smooth scroll? Or go back to the old mechanism?
Flags: needinfo?(dao+bmo)
Comment 17•7 years ago
|
||
We can't use smooth scrolling there in a sensible way since we don't know the scroll destination until the user leaves the scroll button. (The line setting scrollPosition needs to call scrollByPixels instead, though.)
Flags: needinfo?(dao+bmo)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8901371 -
Attachment is obsolete: true
Comment 19•7 years ago
|
||
mozreview-review |
Comment on attachment 8905133 [details] Bug 1387130 - Use original tabstrip scrolling behaviour when using scrollbuttons. https://reviewboard.mozilla.org/r/176934/#review181940 ::: toolkit/content/widgets/scrollbox.xml:721 (Diff revision 1) > this._scrollTimer.cancel(); > this._mousedown = false; > this._scrollIndex = 0; > + > + if (this.smoothScroll) { > + this._arrowScrollAnim.stop(); Is there a reason why you didn't restore _stopScroll to as it was before <https://hg.mozilla.org/mozilla-central/rev/3845af01ea9f#l2.425>?
Attachment #8905133 -
Flags: review?(dao+bmo)
Assignee | ||
Comment 20•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8905133 [details] Bug 1387130 - Use original tabstrip scrolling behaviour when using scrollbuttons. https://reviewboard.mozilla.org/r/176934/#review181940 > Is there a reason why you didn't restore _stopScroll to as it was before <https://hg.mozilla.org/mozilla-central/rev/3845af01ea9f#l2.425>? Thought I needed to keep that arrangement because of the browser_overflowScroll.js test failure that I wrestled with in bug 1356705 - but you're right, not necessary it seems. I've reverted that part too.
Comment hidden (mozreview-request) |
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8905133 [details] Bug 1387130 - Use original tabstrip scrolling behaviour when using scrollbuttons. https://reviewboard.mozilla.org/r/176934/#review182020 Thanks!
Attachment #8905133 -
Flags: review?(dao+bmo) → review+
Comment 23•7 years ago
|
||
Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ad5c7dbaa4d6 Use original tabstrip scrolling behaviour when using scrollbuttons. r=dao
Assignee | ||
Comment 24•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #22) > Comment on attachment 8905133 [details] > Bug 1387130 - Use original tabstrip scrolling behaviour when using > scrollbuttons. > > https://reviewboard.mozilla.org/r/176934/#review182020 > > Thanks! And thank you for the fast review!
Comment 25•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ad5c7dbaa4d6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Comment 26•7 years ago
|
||
For QA: Testing this one should be straight-forward. Ensure you have an overflowed tab strip, lots and lots of tabs, then click and hold the tab scrollbuttons. With this fix, it should be a reasonably constant quick speed in the scrolling. The bad case is when the tab strip "pulses" and goes one tab at a time.
Whiteboard: [photon-performance] → [photon-performance][qa-commented]
Updated•7 years ago
|
status-firefox55:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Comment 27•7 years ago
|
||
The current incarnation works much better, as of old - here I have 57.0a1 (2017-09-12) (32-bit) However, the corollary function - moving a tab outside of current range - still displays the errant slow, hestitant behaviour. I feel this is part of the same bug; do I have agreement? To demonstrate: get lots of tabs up, sufficient that the < scroll arrow appears. lock on a tab with the mouse, attempt to move the tab left, outside of currently visible row of tabs (the move initially seems to hit barriers, and) once it starts working the move is slow & jerky as per 'inchworm' This is close to unusable [ and it is not possible to drag a tab from the drop-down list ]. Moving a tab within the visible range seems fluent & fine. _____ Pursuant to my earlier merged report on this topic, wide web-pages also display less than optimal behaviour, but it may be acceptable. Get a web-page and magnify it ctrl+ until it exceeds the screen-width, and (ideally but not always) the horizontal scroll bar appears. Try to scroll laterally using the -> & <- keys I find the scrolling is slow to start, then moves quite fast (enough) but with noticeable granularity.
Comment 28•7 years ago
|
||
(In reply to Bill his name from comment #27) > However, the corollary function - moving a tab outside of current range - > still displays the errant slow, hestitant behaviour. > > I feel this is part of the same bug; do I have agreement? That sounds like bug 1387718.
Comment 29•7 years ago
|
||
Thanks, Dão I agree with your https://bugzilla.mozilla.org/show_bug.cgi?id=1387130#c28 However I feel these visible bugs may all be rooted in the same place, and would be better fixed at root rather than with sticking plaster. I've moved my additional comments 27 to 1387718.
Comment 30•7 years ago
|
||
Verified on: Windows 10x64(quantum ref.), Windows 7, Windows 8.1, Mac OSX 10.12, Ubuntu 16.04 - with FF57 x64 9/15 and also covered with the pre-beta smoke tests.
Comment 31•7 years ago
|
||
(In reply to Adrian Florinescu [:AdrianSV] from comment #30) > Verified on: Windows 10x64(quantum ref.), Windows 7, Windows 8.1, Mac OSX > 10.12, Ubuntu 16.04 - > with FF57 x64 9/15 and also covered with the pre-beta smoke tests. Thanks, probably now shifted to bug 1387718
You need to log in
before you can comment on or make changes to this bug.
Description
•