1190662 - Browser action popup HTML should be reloaded each time popup is opened

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect, P1)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

This is needed for Adblock Plus.

Comment 1

[Tim Nguyen :ntim]

Could be unloaded when closed for better performance as well.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch

This just moves some of the logic around so that we don't create the panel until it's needed. It also eliminates prevPopups, which is nice.

I also added code to automatically size the popup to fit the content that the add-on provides. This makes it look a lot better.

Finally, I had to add some code to expose the extension ID to the test harness. That's needed so that I can find the CustomizableUI widget ID, which is based on the extension ID.

Comment 4

[Gabor Krizsanits (INACTIVE)]

Comment on attachment 8657907 [details] [diff] [review]
patch

Review of attachment 8657907 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/extensions/ext-browserAction.js
@@ +118,5 @@
> +        browser.removeEventListener("load", contentLoadListener);
> +
> +        let contentViewer = browser.docShell.contentViewer;
> +        let width = {}, height = {};
> +        contentViewer.getContentSize(width, height);

It seems to me that getContentSize can throw... should we handle that?

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/73b8ddd6dac9

Comment 6

[Phil Ringnalda (:philor)]

Backed out the whole push in https://hg.mozilla.org/integration/mozilla-inbound/rev/f2e83a8b6b8e since it was intertangled and I wasn't entirely sure which part caused Mulet mochitest-5 to become permaorange with https://treeherder.mozilla.org/logviewer.html#?job_id=14545777&repo=mozilla-inbound

Comment 8

[Blake Winton (:bwinton) (:☕️)]

I confirm this patch does fix bug 1199048, although when I run it with the WebExtension version of Whimsy from https://github.com/bwinton/whimsy/tree/WebExtensionAPI, it looks like https://www.dropbox.com/s/dl8d94v7pgj65z7/Screenshot%202015-09-23%2019.05.57.png?dl=0 which leads me to believe that the auto-sizing might need some help…  :)

Comment 9

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I'm not seeing a browser action popup at all in that version of whimsy. Are there uncommitted changes?

Comment 10

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Also, the icon links seem to be broken. I don't see the whimsycorn images in the repo.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/52814ef8ceaa

Comment 12

[Blake Winton (:bwinton) (:☕️)]

Ugh, yes there were uncommitted changes.  That should be fixed now.

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/52814ef8ceaa

Comment 16

[Vasilica Tamas, QA [:vtamas]]

Is manual testing necessary here? If yes, could you provide some clear steps and the proper webextension? (I tried to submit on stage the webextension mentioned in Comment 8 but it seems to be incompatible with any platform version - see screenshot: http://screencast.com/t/VJXyQVwdf)

Comment 17

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

The patch contains an automated test so no manual testing is needed.