Closed Bug 1436450 Opened 3 years ago Closed 2 years ago

Unavailable CanvasRenderingContext2D.drawWindow() with "<all_urls>" in "optional_permissions"

Categories

(WebExtensions :: General, defect, P2)

defect

Tracking

(firefox60 fixed, firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox60 --- fixed
firefox61 --- fixed

People

(Reporter: yuki, Assigned: zombie)

Details

Attachments

(3 files)

"<all_urls>" permission is required to capture tab contents by "CanvasRenderingContext2D.drawWindow()"(*), but it still fail if the permission is granted optionally.

(*)https://discourse.mozilla.org/t/drawwindow-in-webextension/13811/4


Steps to reproduce:

 1. Create an addon with "optional_permissions" including "<all_urls>".
 2. Load the addon by "about:debugger".
 3. Call "browser.permissions.request()" to grant "<all_urls>" permission.
 3. Call "CanvasRenderingContext2D.drawWindow()" in a content script.

Expected result:
The operation succeeds and I successfully get a captured image.

Actual result:
"Error: context.drawWindow is not a function" is reported and the operation fails.
After you loaded attached addons "success.xpi" and "failure.xpi", you'll see two toolbar buttons added by them. Click on the "success case" button opens a tab and its content is immediately replaced to an image generated from captured image. On the other hand, click on the "failure case" button also opens a tab but only an error is reported.

There are differences only around codes to grant optional permissions. Actual diff:

--------
$ diff -ur success failure
diff -ur success/background.js failure/background.js
--- success/background.js       2018-02-08 02:59:57.645307300 +0900
+++ failure/background.js       2018-02-08 02:28:50.465952800 +0900
@@ -1,4 +1,7 @@
 browser.browserAction.onClicked.addListener(async () => {
+  const granted = await browser.permissions.request({ origins: ['<all_urls>'] });
+  if (!granted)
+    return;
   const tab = await browser.tabs.create({
     url:    '/background.js',
     active: true
diff -ur success/manifest.json failure/manifest.json
--- success/manifest.json       2018-02-08 02:56:19.327771900 +0900
+++ failure/manifest.json       2018-02-08 02:14:01.473073700 +0900
@@ -1,12 +1,14 @@
 {
   "manifest_version": 2,
-  "name": "success case",
+  "name": "failure case",
   "version": "1",
   "author": "YUKI \"Piro\" Hiroshi",
-  "description": "success case",
+  "description": "failure case",
   "permissions": [
     "activeTab",
-    "tabs",
+    "tabs"
+  ],
+  "optional_permissions": [
     "<all_urls>"
   ],
   "background": {
@@ -15,6 +17,6 @@
     ]
   },
   "browser_action": {
-    "default_title": "success case"
+    "default_title": "failure case"
   }
 }
--------
I don't know it is related to this problem, but in such a situation "browser.tabs.captureTab()" and "browser.tabs.captureVisibleTab()" are also unavailable.
Zombie -- do you mind taking a look at this?
Flags: needinfo?(tomica)
Looking into.
Assignee: nobody → tomica
Status: NEW → ASSIGNED
Flags: needinfo?(tomica)
Priority: -- → P2
Attachment #8962534 - Flags: review?(aswan)
Comment on attachment 8962534 [details]
Bug 1436450 - Fix <all_urls> as an optional API permission

https://reviewboard.mozilla.org/r/231332/#review237338

::: toolkit/components/extensions/ext-permissions.js:37
(Diff revision 3)
> +          if (origins.includes("<all_urls>")) {
> +            permissions.push("<all_urls>");
> +          }

I guess this just gets ignored when we build strings for the permission prompt since we don't have a string for it?  How about just moving this to after the prompt has been accepted instead.
Attachment #8962534 - Flags: review?(aswan) → review+
Pushed by tomica@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3a641d9546c6
Fix <all_urls> as an optional API permission r=aswan
Bah, sorry about that.  Always run the test, no matter how small the change..
Flags: needinfo?(tomica)
Pushed by tomica@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e31b828d6003
Fix <all_urls> as an optional API permission r=aswan
https://hg.mozilla.org/mozilla-central/rev/e31b828d6003
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
The patch seems to need to be uplifted to Firefox 60.

Extensions in Firefox 60 | Mozilla Add-ons Blog
https://blog.mozilla.org/addons/2018/04/02/extensions-firefox-60/

This blog post advertises tabs.captureTab and tabs.captureVisibleTab as:

> The tabs API now supports a tabs.captureTab method which can be passed
> a tabId to capture the visible area of the specified tab (as opposed to
> the tabs.captureVisibleTab API, which only captured the active tab of a
> window).

but my addon cannot use them because this bug blocks to call them with optional permissions. Of course I can add "<all_urls>" to the permanent permissions, but I moved it from the permanent permissions to optional in an old version because I got a feedback like that: "I cannot install this addon due to my company policy, because it requires <all_urls>" permission. If it is not a critical feature, please make the permission optional."
Hi Piro, I'm not sure that's reason enough to uplift, but feel free to nominate for beta, and Julien (or another release driver) will decide.
Comment on attachment 8962534 [details]
Bug 1436450 - Fix <all_urls> as an optional API permission

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: Addons cannot use some features advertised at https://blog.mozilla.org/addons/2018/04/02/extensions-firefox-60/ on Firefox 60
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: No
[Why is the change risky/not risky?]: This patch just migrates a special permission from optional permissions. The added permission is defined as a fixed literal in the code, and incoming string is not used.
[String changes made/needed]: None
Attachment #8962534 - Flags: approval-mozilla-beta?
Comment on attachment 8962534 [details]
Bug 1436450 - Fix <all_urls> as an optional API permission

Simple fix with new test coverage for a new feature being advertised in Fx60. Approved for 60.0b12.
Attachment #8962534 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.