594438 - Extremely slow behavior of the Add-ons Manager in a popup

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b6pre) Gecko/20100908 Firefox/4.0b6pre

If the Add-ons Manager gets opened in a popup the reaction on user inputs is extremely slow.

Steps:
1. Open http://hg.mozilla.org/qa/litmus-data/raw-file/tip/firefox/popups/popups_2.html
2. Let one of the popups show
3. Press Cmd/Ctrl+Shift+A
4. Interact with the Addons Manager

With step 4 you will notice that the AOM is not usable.

Comment 1

[Dave Townsend [:mossop]]

This seems to be an issue with rendering. Looks like I click on something and then it just isn't getting repainted, I can force the repaint by resizing the popup window. I would guess either a problem with retained layers or bug 130078

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I suspect 130078 :-)

Comment 3

[Timothy Nikkel (:tnikkel)]

I don't understand, how do I reproduce? I don't get any addons manager following the steps in comment 0.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I do

Comment 5

[Dave Townsend [:mossop]]

You have to make sure that "Open new windows in a new tab" is unchecked in options and maybe click the popup-blocked thing in the status bar to get the popup to appear

Comment 6

[Timothy Nikkel (:tnikkel)]

I was letting the blocked popup appear already. I tried unchecking "Open new windows in a new tab", still no addons manager.

Comment 7

[Dave Townsend [:mossop]]

(In reply to comment #6)
> I was letting the blocked popup appear already. I tried unchecking "Open new
> windows in a new tab", still no addons manager.

Does cmd/ctrl-shift-a open an add-ons manager normally? (Maybe you have an extension overriding that shortcut)

Comment 8

[Timothy Nikkel (:tnikkel)]

Nevermind, some app was globally stealing the ctrl-shift-a shortcut for itself. Never knew that app was doing that, or that it normally brings up the addons manager.

Comment 9

[Timothy Nikkel (:tnikkel)]

Looks like some js in tabbrowser.xml is failing because some things are not defined and that causes it to not set the document (docshell) as active.

Comment 10

[Timothy Nikkel (:tnikkel)]

Caused by bug 592131, since that made painting depend on the isActive flag.

Comment 11

[Timothy Nikkel (:tnikkel)]

And the failing to set the isActive flag properly predates bug 130078 too.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Is tabbrowser defaulting isActive to false somewhere or something?

Comment 13

[Timothy Nikkel (:tnikkel)]

Yes. See http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#1255

Comment 14

[Timothy Nikkel (:tnikkel)]

I think this is a browser bug. When you close the window with the addons manager it asks you if you want to close 2 tabs. There is no tabbar visible on that window. Using the usual keys like Ctrl-T, Alt-<number>, Ctrl-Tab don't work on the popup window. I think the addons manager shortcut key is going around this and opening a new tab in a window that doesn't expect it. And that is probably causing the problem here.

Comment 15

[Dave Townsend [:mossop]]

We have bug 593687 on file to fix the case of hte add-ons manager itself. The question is whether there is anything else specifically that needs to be fixed. It's surprising to me that we need JS code to manually do something to make painting work.

Comment 16

[Timothy Nikkel (:tnikkel)]

If the javascript code did nothing painting would work because the IsActive flag on the docshell defaults to true. The problem is the js code is setting IsActive to false and then never setting it to true.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Um...  The link in comment 13 shows browser front end manually doing something to make painting NOT "work".  Then before we get to the code in the front end that would switch it back to "working" we get an exception, apparently.  So the front end never makes painting "work" for that tab.

Comment 18

[:Gavin Sharp [email: [e-mail]]]

I didn't really realize that the isActive state is so critical to correct functioning (as opposed to something used for optimizations). The fact that it messes up painting if not set correctly makes it a somewhat dangerous footgun... Front-end isn't really used to that kind of power! :)

In any case, this seems like just a side effect (dupe) of bug 593687/bug 586234.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Well... even before bug 592131, the isActive flag disabled animated images and made any style changes not take effect for a full second.  Optimizations, all, yet arguably "not correct functioning" if you can actually _see_ the relevant document.

Comment 20

[Timothy Nikkel (:tnikkel)]

Hmm, can we not just use the visibility of the views now that 130078 is landed instead of the isActive flag? We have a chase more pointers because we have to go up the view tree looking for visibility info, but the result should be the same, no?

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We could use view visibility here.

However, when we move to display-list-based invalidation, bug 592131 will be a non-issue; changes in background tabs will never cause invalidation simply because the content is not visible before and after the paint. So we can revert bug 592131, effectively, and this bug would then not be possible; we'd repaint content correctly even if it's in a visible non-isActive tab.

Comment 22

[Boris Zbarsky [:bzbarsky]]

> but the result should be the same, no?

Not for e10s, right?  Also not for other embeddings?

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah, that's true.

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Moving this to Addons-Manager since it's a chrome code bug. Probably fixed by 593687 anyway.

Comment 25

[Justin Dolske [:Dolske]]

Unowned, over to Blair to handle.

Comment 26

[Dave Townsend [:mossop]]

Moving this to browser frontend since the problem is apparently that tabbrowser isn't doing the right thing in this case. I'm not entirely sure this needs an owner though since bug 593687 will stop this case from coming up.

Comment 27

[Dave Townsend [:mossop]]

Sine bug 593687 we shouldn't be able to open in a popup any longer so this should be fixed.

Comment 28

[George Carstoiu]

Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110215 Firefox/4.0b12pre

Issue was verified and it's no longer present.