1493396 - permissions.request() does not work in sidebar, rejected with Error: An unexpected error occurred

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P2)

Description

[Rob Wu [:robwu]]

Attachment: sidebar-perm.zip

STR.
1. Load attached extension, e.g. at about:debugging.
   The extension opens a sidebar. If you don't see this, open it via the menu: Views > Sidebar > "Sidebar with optional permissions"
2. Click on the pfirst ermissions.request (<all_urls>) button in the sidebar.
3. Expect a permission prompt. Grant the permissions.
4. Look at the output in the sidebar.

5. Visit example.com and click on the extension button.
6. Expect a dialog showing "undefined".

7. Click on the permissions.request (tabs) button in the sidebar.
8. Expect a dialog showing the tab's URL.

Expected result:
- Expecting a permission prompt at step 3.
- After step 4, expecting:
  [result] permissions.request( ..<all_urls>..) = true
- Step 6 and 8 show the expected dialogs.

Actual result:
- After step 4, actual output is:
  [error] permissions.request( ..<all_urls>..) = Error: An unexpected error occurred
- Step 6 and 8 fail (because the permissions were not granted).
- The console shows the following error after step 4 (from Nightly 64, buildID 20180914100156 ):

> Argument 1 of ChromeUtils.getClassName is not an object. PopupNotifications.jsm:218
> window.PopupNotifications is null; can't access its "show" property ExtensionsUI.jsm:344 


Note: The expected result occurs when permission prompts are disabled, by setting extensions.webextOptionalPermissionPrompts to false at about:config

Comment 1

[Matthew N. [:MattN]]

FYI you can use the *popupnotificationanchor attribute on a <browser> to anchor popupnotifications on a different element, like something in the sidebar: https://dxr.mozilla.org/mozilla-central/rev/56b988a937689d5599400afa59b72c390b40abf2/toolkit/modules/PopupNotifications.jsm#19,40-53

Comment 2

[Rob Wu [:robwu]]

What is the desired behavior for permission doorhangers in sidebars?

1. Current - they do not appear (=this bug).
2. Anchored to the active tab (=usual behavior for doorhangers).
3. Anchored to a different location, such as the sidebar header.

Comment 3

[Shane Caraveo (:mixedpuppy)]

The sidebar header is a perfectly reasonable place to put it (and really the only place).  It should anchor on the icon like the menu dropdown does.  Luca should be aware since the sidebar code is shared with the devtools sidebar.

Comment 4

[emanuela [ux]]

My recommendation right now is to go with the second solution. So anchored to the active tab, which is also the current standard behavior for all the other permissions.

If we're going to change the way we're messaging users in the future, this may change as well. However, at the moment there is not enough evidence this will happen shortly.

Comment 5

[Jim Mathies [:jimm]]

Leaving a P2, missing permission prompt causing breakage sidebar extensions.

Comment 6

[Rob Wu [:robwu]]

permissions.request is implemented here: https://searchfox.org/mozilla-central/rev/919607a3610222099fbfb0113c98b77888ebcbfb/toolkit/components/extensions/parent/ext-permissions.js#34-99

It relies on the context.pendingEventBrowser || context.xulBrowser; member to find the <browser> . This <browser> is where the notification is shown, which is currently only implemented for tabs, not for sidebars. To fix this bug, we should use the selected tab of the sidebar instead (in the window where the sidebar is shown).

To resolve this bug, you need to find when the sidebar's <browser> is used. The element can be identified by one of its attributes (the element is created at https://searchfox.org/mozilla-central/rev/919607a3610222099fbfb0113c98b77888ebcbfb/browser/base/content/webext-panels.js#36-45). The Browser Toolbox can be used directly debug the sidebar and its internals - see https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox

Once you're able to identify calls from the sidebar, get the top window and find the selected tab browser. Hint: The top-level window containing the sidebar can be found via the browser.ownerGlobal.windowRoot property.

To make sure that the functionality works as expected, unit tests should be written. E.g. based on the manual test from the bug report.

Comment 7

[Falguni Islam]

Hello, I am new to contributing to Bugzilla can you assigned me to this issue so that I can learn to fix these bugs and also suggest to me how can Is start working on this bug to fix it.

Comment 8

[Rob Wu [:robwu]]

(In reply to Falguni Islam from comment #7)

Hello, I am new to contributing to Bugzilla can you assigned me to this issue so that I can learn to fix these bugs and also suggest to me how can Is start working on this bug to fix it.

Hello, coincidentally someone else has started on a patch that may also address this bug (https://phabricator.services.mozilla.com/D98574). I suggest that you try a different bug. To get started, see the guide at https://wiki.mozilla.org/WebExtensions/Contribution_Onramp (this page also links to a list of other bugs where you can choose from).

Comment 10

[Rob Wu [:robwu]]

This bug can also be triggered when permissions.request is called in response to commands.onCommand from the background page (in which case context.pendingEventBrowser is null and context.xulBrowser is the background <browser>.

window is the hidden window (https://searchfox.org/mozilla-central/rev/59e797b66f5ce8a27ede0e7677688931be7aed20/toolkit/components/extensions/ExtensionParent.jsm#1137), which doesn't have a UI.

Comment 11

[max]

Notice that if keystroke is assigned to browser action then everything works right so window is that code path is correct. The problem happens when a shortcut is defined namely for user command handled by commands.onCommand. So siblings entries from extension manifest behaves quite differently (from Bug #1721428 marked as a duplicate of this one). I have not tried page action though.

Comment 12

[max]

There are some other bugs related to context of permission popup: Bug #1679925 for inactive tab context menu and mention of this issue in Bug #1433292 comment 14.

Comment 13

[Vaidehi A]

Hello. This is Vaidehi. I am a new contributor. Can I work on this issue?

Comment 14

[kernp25]

(In reply to Vaidehi A from comment #13)

Hello. This is Vaidehi. I am a new contributor. Can I work on this issue?

Comment 15

[Rob Wu [:robwu]]

Hi Vaidehi, sure you can. Have you read the context in this bug report, comment 6 in particular?

Feel free to ask questions if you have doubts.

Comment 16

[Vaidehi A]

Thank you Rob. I was able to reproduce this. But, I haven't really gone through comment 6 context. Will work on it and let you know.

Comment 17

[Vaidehi A]

Hi Rob.
First-of-all, there are some differences which I observed in the expected results mentioned by you.

• No matter what the value of extensions.webextOptionalPermissionPrompts is, I never get a permission prompt for step 3. However, I don't get the error when extensions.webextOptionalPermissionPrompts is set to false, when I click on permissions.request (<all_urls>).
• When extensions.webextOptionalPermissionPrompts is false, at step 6 pop up I get the tab's url instead of undefined.

From what I have understood, I think here the bug is that request for permissions is not being created for sidebars. For that, we need to find <browser> element for sidebar. However, I am not getting how to do it exactly.

To resolve this bug, you need to find when the sidebar's <browser> is used. The element can be identified by one of its attributes (the element is created at https://searchfox.org/mozilla-central/rev/919607a3610222099fbfb0113c98b77888ebcbfb/browser/base/content/webext-panels.js#36-45). The Browser Toolbox can be used directly debug the sidebar and its internals - see https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox

Can you explain it a little - how to identify when the sidebar's <browser> is used and how to use attributes for it (using browser toolbox)?

Comment 18

[Rob Wu [:robwu]]

(In reply to Vaidehi A from comment #17)

Hi Rob.
First-of-all, there are some differences which I observed in the expected results mentioned by you.

• No matter what the value of extensions.webextOptionalPermissionPrompts is, I never get a permission prompt for step 3. However, I don't get the error when extensions.webextOptionalPermissionPrompts is set to false, when I click on permissions.request (<all_urls>).

That observation is this bug. A prompt should be seen, but because the prompt UI cannot be anchored to the right <browser> element, the prompt doesn't appear.

• When extensions.webextOptionalPermissionPrompts is false, at step 6 pop up I get the tab's url instead of undefined.

With extensions.webextOptionalPermissionPrompts set to false, the prompt UI is skipped and any requests are silently approved. Getting the tab's URL is what would happen if the prompt were to show up, followed by the user approving the prompt.

From what I have understood, I think here the bug is that request for permissions is not being created for sidebars. For that, we need to find <browser> element for sidebar. However, I am not getting how to do it exactly.

To resolve this bug, you need to find when the sidebar's <browser> is used. The element can be identified by one of its attributes (the element is created at https://searchfox.org/mozilla-central/rev/919607a3610222099fbfb0113c98b77888ebcbfb/browser/base/content/webext-panels.js#36-45). The Browser Toolbox can be used directly debug the sidebar and its internals - see https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox

Can you explain it a little - how to identify when the sidebar's <browser> is used and how to use attributes for it (using browser toolbox)?

At the start of comment 6, I pointed to the code where the permissions.request handler is implemented. With the Browser Toolbox, you can put a breakpoint after the line where the browser variable is initialized, and trigger the permissions.request call via the test case. After triggering the test case, the breakpoint will be triggered and you can use the console to examine the browser variable and anything related to it to get a better understanding of how everything works.

As you can see from the code in webext-panels.js, the sidebar is hosted in a <browser> with the "webext-panels-browser" ID. So a way to identify the browser is to check the condition browser.id === "webext-panels-browser".

Then, as mentioned at the bottom of comment 6, the other part of the solution is

Once you're able to identify calls from the sidebar, get the top window and find the selected tab browser. Hint: The top-level window containing the sidebar can be found via the browser.ownerGlobal.windowRoot property.

The snippet above shows how one can get the top-level window containing the sidebar, the part of getting the selected tab browser is to read the .gBrowser.selectedBrowser property of it.

Comment 19

[max]

(In reply to Rob Wu [:robwu] from comment #18)

With extensions.webextOptionalPermissionPrompts set to false, the prompt UI is skipped and any requests are silently approved.

The cited phrase reminded me the Bug #1744960 with the prompt for external scheme handler (regular page vs. add-on discrepancy of behavior).

Comment 20

[Rob Wu [:robwu]]

(In reply to max from comment #19)

(In reply to Rob Wu [:robwu] from comment #18)

With extensions.webextOptionalPermissionPrompts set to false, the prompt UI is skipped and any requests are silently approved.

The cited phrase reminded me the Bug #1744960 with the prompt for external scheme handler (regular page vs. add-on discrepancy of behavior).

That referenced bug is not related at all. This preference defaults to true and can be set to false for testing purposes.

Comment 21

[max]

(In reply to Rob Wu [:robwu] from comment #20)

(In reply to max from comment #19)

(In reply to Rob Wu [:robwu] from comment #18)

With extensions.webextOptionalPermissionPrompts set to false, the prompt UI is skipped and any requests are silently approved.

The cited phrase reminded me the Bug #1744960 with the prompt for external scheme handler (regular page vs. add-on discrepancy of behavior).

That referenced bug is not related at all. This preference defaults to true and can be set to false for testing purposes.

Thank you for the clarification. Almost certainly I was wrong and I added more confusion by the imprecise quote. I never thought that extensions.webextOptionalPermissionPrompts might be relevant. Actually I associated "because the prompt UI cannot be anchored to the right <browser> element, the prompt doesn't appear." and "any requests are silently approved" with the external scheme handler popup. Sorry for the noise.

Comment 22

[kernp25]

(In reply to Rob Wu [:robwu] from comment #10)

This bug can also be triggered when permissions.request is called in response to commands.onCommand from the background page (in which case context.pendingEventBrowser is null and context.xulBrowser is the background <browser>.

window is the hidden window (https://searchfox.org/mozilla-central/rev/59e797b66f5ce8a27ede0e7677688931be7aed20/toolkit/components/extensions/ExtensionParent.jsm#1137), which doesn't have a UI.

Then we must use windowTracker.topWindow to get the browser? Because, browser.ownerGlobal.windowRoot doesn't work in this case!

Comment 23

[Rob Wu [:robwu]]

windowTracker.topWindow would return the active browser window;
If you are interested in the browser element, use tabTracker.activeTab.linkedBrowser instead.

Comment 24

[kernp25]

I mean, can the patch use windowTracker.topWindow.gBrowser.selectedBrowser to also fix the bug in comment 10?

Or is this bug only for the sidebar?

Because, you said we must check for browser.id === "webext-panels-browser"?

Comment 25

[Rob Wu [:robwu]]

A sidebar is always associated with a window, so it would make more sense to use that window.
In practice, a permission request from a sidebar will end up in the same window as the topWindow, but in theory it's possible for the window focus to change before the permission prompt appears.

As for gBrowser, that's a desktop-only property. The snippet that I shared in comment 23 is the general code that can be used for code shared between desktop and mobile code. (permissions.request doesn't work on mobile atm, but that's actively being worked on in bug 1601420.

Comment 26

[kernp25]

Attachment: Bug 1493396 - Make browser.permissions.request work in WebExtension sidebar. r?robwu

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3582cf5b4523
Make browser.permissions.request work in WebExtension sidebar. r=robwu

Comment 28

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/3582cf5b4523