Closed Bug 1408152 Opened 4 years ago Closed 4 years ago

Throbbers get out of sync when dragging tabs

Categories

(Firefox :: Theme, defect, P5)

57 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox57 --- wontfix
firefox59 --- fixed

People

(Reporter: abr, Assigned: jaws)

Details

Attachments

(1 file)

If you drag a tab while it has a loading throbber in it, the throbber loses sync with other throbbers in the same window.

STR:

- Create a bookmark folder with a fairly large number of tabs (e.g. 20 or so)
- From that folder, perform an "Open All in Tabs" 
- While the pages are still loading, drag one tab to any location other than its starting one
- Observe that throbber on the dragged tab is no longer in sync with the other tabs' throbbers

(I discovered this in a less contrived but harder-to-reproduce situation involving only two tabs -- so it does arise even without a large number of tabs loading at the same time)
Priority: -- → P5
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #8929061 - Flags: feedback?(bbirtles)
Comment on attachment 8929061 [details]
Bug 1408152 - Sync the throbber animations again after the tab has finished moving.

I'd be curious to know what re-starts the animation. Perhaps something clobbers the animation-name property? We could set a breakpoint in nsAnimationManager.cpp to find out or even just listen for 'animationcancel' events (assuming the animation is actually being re-created).

In any case, _syncThrobberAnimations should be safe to call repeatedly so this seems fine to me.
Attachment #8929061 - Flags: feedback?(bbirtles) → feedback+
Comment on attachment 8929061 [details]
Bug 1408152 - Sync the throbber animations again after the tab has finished moving.

https://reviewboard.mozilla.org/r/200364/#review205672

Yeah, looks like this could be pulled out as a method that just takes a single tab as a param? That seems cleaner than this...

Otherwise this looks OK to me given bbirtles is happy? Up to you if you want to re-request review and/or do more digging.
Attachment #8929061 - Flags: review?(gijskruitbosch+bugs) → review+
I pulled out the function and put it on tabbrowser with a `tab` argument. I'll land this now since the refactoring is pretty straightforward.
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f020bc218c8b
Sync the throbber animations again after the tab has finished moving. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/f020bc218c8b
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in before you can comment on or make changes to this bug.