Closed Bug 1691144 Opened 2 months ago Closed 2 months ago

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

Categories

(WebExtensions :: General, defect, P3)

Firefox 86
defect

Tracking

(firefox86 fixed, firefox87 fixed)

RESOLVED FIXED
87 Branch
Tracking Status
firefox86 --- fixed
firefox87 --- fixed

People

(Reporter: bugzilla, Assigned: Dexter, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(1 file, 1 obsolete file)

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.

Severity: -- → S3
Keywords: good-first-bug
Priority: -- → P3
Mentor: tomica

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

Attached patch Patch (obsolete) — Splinter Review

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

Assignee: nobody → bugzilla
Flags: needinfo?(rhelmer)

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.

Assignee: bugzilla → alessio.placitelli
Flags: needinfo?(rhelmer)
Attachment #9202122 - Attachment is obsolete: true

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.

Pushed by aplacitelli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/076c2a071ace
Trigger windows.onFocusChanged with no private window permission. r=zombie
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch

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:
Attachment #9202393 - Flags: approval-mozilla-beta?

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

Approved for our last beta, thanks.

Attachment #9202393 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.