Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Description • 9 years ago

This is needed for Adblock Plus.

	Tim Nguyen :ntim
	Comment 1 • 9 years ago

Could be unloaded when closed for better performance as well.

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 2 • 9 years ago

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.

	Gabor Krizsanits (INACTIVE)
	Comment 4 • 9 years ago

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?

	Pulsebot
	Comment 5 • 9 years ago

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

	Phil Ringnalda (:philor)
	Comment 6 • 9 years ago

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

	Blake Winton (:bwinton) (:☕️)
	Comment 8 • 9 years ago

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…  :)

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 9 • 9 years ago

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

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 10 • 9 years ago

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

	Pulsebot
	Comment 11 • 9 years ago

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

	Blake Winton (:bwinton) (:☕️)
	Comment 12 • 9 years ago

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

	Carsten Book [:Tomcat]
	Comment 13 • 9 years ago

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

	Vasilica Tamas, QA [:vtamas]
	Comment 16 • 8 years ago

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)

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 17 • 8 years ago

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