Closed Bug 1202483 Opened 7 years ago Closed 7 years ago

Support ImageData for browser.browserAction.setIcon

Categories

(WebExtensions :: Untriaged, defect)

defect
Not set
normal

Tracking

(firefox44 fixed)

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: billm, Assigned: billm)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
The tricky part here is that the ImageData must be rendered into a canvas that's same-origin with the ImageData. To do that, we need to create a windowless docshell with the extension's principal and create a <canvas> inside of it.

One problem here is that the GlobalManager code assumes that any docshell created with extension principals must be a tab that's showing extension content. That's not the case here, so I added a call to GlobalManager.injectInDocShell with a null context. That causes the icon docshell to be ignored.

I also moved some common test code into head.js.
Attachment #8657938 - Flags: review?(gkrizsanits)
Comment on attachment 8657938 [details] [diff] [review]
patch

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

The patch is correct, but please consider my comment below...

::: browser/components/extensions/ext-browserAction.js
@@ +273,5 @@
> +function convertImageDataToPNG(extension, imageData)
> +{
> +  let webNav = imageRendererMap.get(extension);
> +  if (!webNav) {
> +    webNav = Services.appShell.createWindowlessBrowser(false);

I don't quite get why do we have to create a docshell for this. Shouldn't the caller context of this API always have a docshell/document with this principal that we can use here to create the canvas?
Attachment #8657938 - Flags: review?(gkrizsanits) → review+
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #2)
> I don't quite get why do we have to create a docshell for this. Shouldn't
> the caller context of this API always have a docshell/document with this
> principal that we can use here to create the canvas?

Yes, there will be a document for the background page, but I don't want to create a canvas element there since it would be visible to the extension.
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
https://hg.mozilla.org/mozilla-central/rev/9fcec26b5327
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.