1679925 - Permission requesting popup is unexpectedly bound to the context tab instead of the active tab when permissions.request() is called from the context menu on a background tab

Status: ASSIGNED

(WebExtensions :: General, enhancement, P3)

Description

[YUKI "Piro" Hiroshi]

Attachment: testcase.xpi: a testcase

The permission requesting popup looks to be bound to the context tab instead of the active tab, even if the requested permission is not related to a specific website (e.g. bookmarks). This causes a non-intuitive result when we request a permission from the tab context menu.

Steps to reproduce

1. Start Firefox.
2. Open multiple tabs in a window.
3. Go to about:debugging.
4. Load the attached testcase.xpi as a temporary extension.
5. Right-click on a background tab.
6. Choose "Request Permission".

Actual result

• Nothing happens. The permission requesting popup does not appear.
• When I close the context menu and activate the background tab, the permission requesting popup appears for the tab.

Expected result

• The permission requesting popup appears immediately.

Environment

• Nightly 85.0a1 (build id: 20201130215246) on Windows 10 Pro 20H2
• Firefox 83.0 (build id: 20201126174332) on ArchLinux

Compatibility note

Google Chrome doesn't support contexts:['tab'] type context menu, so this is quite Firefox specific issue.

Comment 1

[YUKI "Piro" Hiroshi]

Codes binding to the permission prompt to the context tab instead of the active tab:
https://searchfox.org/mozilla-central/rev/0d6e8b21569f93a1e1ae8e377ab10f43a6cb12c1/toolkit/components/extensions/parent/ext-permissions.js#71
https://searchfox.org/mozilla-central/rev/0d6e8b21569f93a1e1ae8e377ab10f43a6cb12c1/browser/components/extensions/parent/ext-menus.js#1340

The method to show the permission prompt:
https://searchfox.org/mozilla-central/rev/0d6e8b21569f93a1e1ae8e377ab10f43a6cb12c1/browser/modules/ExtensionsUI.jsm#365
https://searchfox.org/mozilla-central/rev/0d6e8b21569f93a1e1ae8e377ab10f43a6cb12c1/toolkit/modules/PopupNotifications.jsm#534
It requires a browser to bind the prompt.

This design looks to be based on a usecase for permissions requested from webpages, e.g. geolocation. Non-host permissions look to unmatch to this design. I think it needs to be redesigned for such non-host permissions.

Comment 2

[YUKI "Piro" Hiroshi]

Or, binding the prompt to the active tab instead of the context tab looks solve this bug. This plan is simpler than a fundamental fix I thought at the previous comment.

Comment 3

[YUKI "Piro" Hiroshi]

Attachment: Bug 1679925 - WIP: Show permission prompt as an appmenu notification

Comment 4

[Rob Wu [:robwu]]

(In reply to YUKI "Piro" Hiroshi from comment #1)

This design looks to be based on a usecase for permissions requested from webpages, e.g. geolocation. Non-host permissions look to unmatch to this design. I think it needs to be redesigned for such non-host permissions.

The pendingEventBrowser design is based on the assumption that the user interaction that counts towards the user interaction requirement of permissions.request is related to the browser. This works for the pageAction/browserAction button, and several context menus (in content areas in a tab, pageAction/browserAction popup, etc). It doesn't work well when the <browser> isn't the (active) tab browser, such as in this bug, where the <browser> can be any background tab, or bug 1493396, where the <browser> is in a sidebar.

I think that the best way to fix this specific bug is to update ext-menus.js, to not call withPendingBrowser for menu events that are unrelated to content.

PS. There have been several bugs related to the tracking of <browser>. See https://bugzilla.mozilla.org/show_bug.cgi?id=1433292#c14 for a list of bugs, to see what potential issues can be faced with the implementation of it.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 6

[Suhaib Mujahid [:suhaib]]

Sorry, there was a problem with the detection of inactive users. I'm reverting the change.