1691144 - windows.onFocusChanged does not fire for private windows when there is no private window permission

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P3)

Description

[Jonathan Mayer]

Expected Behavior

If the user switches from an ordinary browsing window to a private browsing window, and the add-on does not have permission for private browsing windows, the windows.onFocusChanged event should fire with the special value windows.WINDOW_ID_NONE (i.e., -1). This behavior allows the add-on to know that the user has switched focus from the ordinary browsing window, without informing the add-on about the existence of the private browsing window. I verified that Chrome currently implements this behavior.

Current Behavior

In the circumstance described above, Firefox does not fire the windows.onFocusChanged event. As a result, the add-on does not learn that the user has switched focus from the ordinary browsing window.

Steps to Reproduce

1. Create an add-on that lacks private browsing window permissions (e.g., by setting incognito: "not_allowed" in the manifest).
2. Add a background script that listens for the windows.onFocusChanged event. For example: browser.windows.onFocusChanged.addListener(windowId => { console.log(`windows.onFocusChanged: ${windowId}`); });
3. Create an ordinary browsing window and a private browsing window.
4. Switch from the ordinary browsing window to the private browsing window.

Proposed Fix

The issue appears to be in the implementation of onFocusChanged in ext-windows.js: https://searchfox.org/mozilla-central/source/browser/components/extensions/parent/ext-windows.js#82

In the function that fires the onFocusChanged event, if the event is for a private browsing window and the add-on does not have permission for private browsing windows, the function returns without firing the event.

if (!context.canAccessWindow(window)) {
  return;
}
windowId = windowTracker.getId(window);

A simple fix would be replacing the return with setting windowId to WINDOW_ID_NONE.

if (!context.canAccessWindow(window)) {
  windowId = Window.WINDOW_ID_NONE;
}
else {
  windowId = windowTracker.getId(window);
}

Proposed Test

The right place for a test looks like browser_ext_windows_incognito.js: https://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_windows_incognito.js

The test could simply create an ordinary browsing window, switch focus to that window, then switch focus to the private browsing window and confirm that onFocusChanged fired with WINDOW_ID_NONE.

Comment 1

[james]

Hey, I would like to try my hands on this bug, I am quite new to contributing on mozilla code base though

Comment 2

[Jonathan Mayer]

Attachment: Patch

In the interest of avoiding duplication of effort: I've already put together a patch (attached), rhelmer is kindly sanity checking.

Comment 3

[Alessio Placitelli [:Dexter]]

Thank you Jonathan! I'll take it from here to speed things up.

Grabbing this as this is a high priority item for the Rally team.

Comment 4

[Alessio Placitelli [:Dexter]]

Attachment: Bug 1691144 - Trigger windows.onFocusChanged with no private window permission. r?zombie

Credits to Jonathan Mayer for submitting the original patch.
The patch changes the onFocusChanged extension event handler
so that it no longer bails out if focus is moving to a private
window, ensuring that the onFocusChanged event gets generated
with the proper WINDOW_ID_NONE identifier.

Comment 5

[Alessio Placitelli [:Dexter]]

Try push: https://treeherder.mozilla.org/jobs?repo=try&revision=968d6c96db8293d74d32e5ad39389073a6ea40e3

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/076c2a071ace
Trigger windows.onFocusChanged with no private window permission. r=zombie

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/076c2a071ace

Comment 8

[Alessio Placitelli [:Dexter]]

Comment on attachment 9202393 [details]
Bug 1691144 - Trigger windows.onFocusChanged with no private window permission. r?zombie

Beta/Release Uplift Approval Request

• User impact if declined: The window.onFocusChanged API will remain broken for the cases of switching to a private window. This is a blocker for releasing Rally studies at launch.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch is minimal and the scope of this change is restricted to a minimal portion of the webextension APIs.
• String changes made/needed:

Comment 9

[Pascal Chevrel:pascalc]

Comment on attachment 9202393 [details]
Bug 1691144 - Trigger windows.onFocusChanged with no private window permission. r?zombie

Approved for our last beta, thanks.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/603959ffc42b