483367 - throbber keeps a' throbbin'

Status: RESOLVED WONTFIX

(Toolkit :: Add-ons Manager, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

In the "Get Add-ons" tab of the extensions manager, the image for the selected addon is shown.  While that image is loading, a throbber is shown behind it.  However, once the image is loaded, the throbber is never removed -- it keeps throbbing away, causing repaints a few times per second for the region where it would be displayed.

The relevant code seems to be here:

http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/extensions.xml#535

It might not do the behaviour I described on trunk since decks always get native widgets for each child and it's easy to tell that the display is not visible, but once we get rid of native widgets, and in current fennec, it causes the massive repaint issues.  I'm not sure if on trunk we'll invalidate (even if we never paint), though; would be good to just get rid of the problem entirely.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Same probem is actually present anywhere where the same approach is used (throbber in a deck), e.g. in the Themes tab.

Comment 2

[Dave Townsend [:mossop]]

I assume just making the image hidden would solve the problem, or would be need to change it to a static image for it to go away?

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Nope, making it hidden should solve the problem.  I'm not sure if the timers will keep firing though, but we should at least not draw it.

Comment 4

[Dave Townsend [:mossop]]

Attachment: style patch

So this is the patch that I would probably look at using, low impact just uses style rules to hide the image. I haven't tested that this definitely works yet but it should.

However now that I've looked at that it seems kind of silly that the deck widget doesn't just effectively do this itself. Regardless of whether it uses native widgets or not does it really have no way to effectively hide the invisible children?

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Yeah, it could very well be a more core problem.. in theory those should be hidden!  So it could just be that we're not stopping invalidation for hidden animated images, which would be bad.

Comment 6

[Dave Townsend [:mossop]]

So ideally we should just fix this in core. However if that isn't a possibility for the 1.9.1 branch and it is a real issue for Fennec then we can just take this patch there as a wallpaper measure. I should be able to verify that the style rules are applying but I'm not sure how to verify whether we still see repaints, would checking that no MozAfterPaints are dispatched be enough?

Comment 7

[Markus Stange [:mstange]]

The core fix is in bug 483841.

Comment 8

[Markus Stange [:mstange]]

Oh, I misread the patch... it doesn't know anything about decks, it only looks at the visibility style; so it won't fix this bug, I think.

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 367373 [details] [diff] [review]
style patch

Can all of that stuff go into toolkit/mozapps/extensions/content/extensions.css?

Comment 10

[Dave Townsend [:mossop]]

(In reply to comment #9)
> (From update of attachment 367373 [details] [diff] [review])
> Can all of that stuff go into
> toolkit/mozapps/extensions/content/extensions.css?

It could, we have a load of stuff in the theme styles that really should be moved into content, I just haven't got around to it yet.

Comment 11

[Stuart Parmenter]

mfinkle: is this an issue for fennec with the new addons manager?

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Fennec doesn't do this in the add-ons manager

Comment 13

[Dave Townsend [:mossop]]

Fennec now uses its own add-ons manager UI and so shouldn't suffer this problem there, certainly no changes to the toolkit add-ons manager will help. I don't think there is a need to fix this for normal Firefox for now so I'm going to close this. Ideally if there is areal underlying problem then that should be fixed in core rather than trying to catch all the cases around that might cause it.