Closed Bug 1729395 Opened 3 years ago Closed 3 years ago

Firefox Multi-Account Containers's "Always Open This Site in" does not work after bug 1708243 part 2

Categories

(WebExtensions :: General, defect)

Firefox 93
defect

Tracking

(Fission Milestone:MVP, firefox-esr78 unaffected, firefox-esr91 unaffected, firefox92 unaffected, firefox93+ verified, firefox94 verified)

VERIFIED FIXED
94 Branch
Fission Milestone MVP
Tracking Status
firefox-esr78 --- unaffected
firefox-esr91 --- unaffected
firefox92 --- unaffected
firefox93 + verified
firefox94 --- verified

People

(Reporter: Fanolian+BMO, Assigned: zombie)

References

(Regression)

Details

(Keywords: nightly-community, regression, reproducible, Whiteboard: [addons-jira] fission-soft-blocker)

Attachments

(4 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:94.0) Gecko/20100101 Firefox/94.0
Build ID: 20210906214243

The bug was originally reported by bbhtt in https://github.com/mozilla/multi-account-containers/issues/2115.
Since it is affecting a "built by Firefox" extension, I think it is better to file the bug here as well so the issue (either in Firefox or MAC) can be fixed before bug 1708243 hits release (it will be in beta in few days).

Steps to reproduce

  1. Install Firefox Multi-Account Containers (MAC) in a new Profile.
  2. Open https://example.com
  3. Click the MAC's toolbar icon (and skip all the first-run intros.) -> Always Open This Site in… -> select "Personal" container.

Actual result

The tab does not reopen in a "Personal" tab.
If I check in MAC's icon -> Manage Containers -> "Personal" -> `Manage Site List…", "example.com" is not added to the list.
In Browser Console, it shows:

Promise rejected while context is inactive: WebExtension context not found! utils.js:117
alwaysOpenInContainer moz-extension://[MAC extension ID]/js/utils.js:117

Expected result

The tab instantly reopens in the "Personal" container and "example.com" is added to the list.

Regression

Last good Nightly: 2021-08-31
First bad Nightly: 2021-09-01
pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e8a630d41a7755dbc6c19093f96a89d8fd351b10&tochange=18d7ddf67682e3eb75376f56e4f014c81d2596b3
Changelist: https://mrotherguy.github.io/fx-nightly-changelog/?date=2021-09-01

Bisecting autoland builds:
pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ab5b37f83023f926201c2b226e35076910ce6fd4&tochange=2dc4f1baccc82806eb203617b0184fa634fea3e5

This is regressed by Bug 1708243 - Stop depending on framescripts with ExtensionGlobal, particularly part 2 of the patch.

Flags: needinfo?(tomica)
Has Regression Range: --- → yes
Has STR: --- → yes
Regressed by: 1708243

(In reply to Fanolian from comment #0)

In Browser Console, it shows:

Promise rejected while context is inactive: WebExtension context not found! utils.js:117
alwaysOpenInContainer moz-extension://[MAC extension ID]/js/utils.js:117

The above error only showed once in my main profile. The actual error is:

Promise rejected after context unloaded: Actor 'Conduits' destroyed before query 'RuntimeMessage' was resolved utils.js:117
alwaysOpenInContainer moz-extension://[MAC extension ID]/js/utils.js:117

Version: Firefox 94 → Firefox 93

This bug can be reproduced even if Fission is disabled.

Thanks for the report and the quick bisection.

Assignee: nobody → tomica
Flags: needinfo?(tomica)
Whiteboard: addons-jira
Whiteboard: addons-jira → [addons-jira]

Same problem here 93.0b2(64) Win10

Status: UNCONFIRMED → ASSIGNED
Fission Milestone: --- → MVP
Ever confirmed: true
Whiteboard: [addons-jira] → [addons-jira] fission-soft-blocker

The issue is that extension is sending a message from the popup to the background page, and closing the popup immediately. Since Bug 1708243 part 2, we're reading sender information in the parent process, but by the time we try to do that, both the browser and parent context are already gone (closed), so we can't read that info.
https://searchfox.org/mozilla-central/rev/aa46c2dccc/toolkit/components/extensions/ExtensionParent.jsm#303

The patch reads the sender url when the Messenger is created, and guards against the browser being undefined.
https://treeherder.mozilla.org/jobs?repo=try&revision=29df3fd901

I'm having issues reducing the extension to a test case, but manually verified the fix, so will land the fix now and do a followup for the test.

Keywords: leave-open
Pushed by tjovanovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3bd81b9fb804
Handle message sender going away during message processing r=robwu
Attachment #9240543 - Attachment description: WIP: Bug 1729395 - Tests that don't fail without the patch → Bug 1729395 - Regression test for sendMessage after a popup is closed! r?robwu

(In reply to Tomislav Jovanovic :zombie from comment #8)

I'm having issues reducing the extension to a test case.

The reason was that MAC tries to send the message after closing the popup, because there is no await for Utils.alwaysOpenInContainer calls in these two places:

https://github.com/mozilla/multi-account-containers/blob/4b56a2f0bb/src/js/pageAction.js#L24-L25
https://github.com/mozilla/multi-account-containers/blob/081ea0bed/src/js/popup.js#L1225-L1226

I'm guessing this was not intentional, as this is a corner case that was never guaranteed to work, so I'm recommending them to fix it at the related issue:
https://github.com/mozilla/multi-account-containers/issues/2115

Keywords: leave-open
Pushed by tjovanovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c8dc9d443c4a
Regression test for sendMessage after a popup is closed! r=robwu

Just verified the original STR is fixed in the latest Nightly.

Hey Fanolian, I don't use MAC, so not sure about other functionality, can you please verify everything else is working as expected?

Flags: needinfo?(Fanolian+BMO)

Comment on attachment 9240420 [details]
Bug 1729395 - Handle message sender going away during message processing r?robwu

Beta/Release Uplift Approval Request

  • User impact if declined: Mozilla's Multi Account Containers, and potentially some other extensions might be partially broken.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 0.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It's a minor change of when and where we capture message sender information, plus a simple null guard. That code has fairly good test coverage.
  • String changes made/needed: none
Attachment #9240420 - Flags: approval-mozilla-beta?
Attachment #9240543 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 94 Branch

(In reply to Tomislav Jovanovic :zombie from comment #13)

Just verified the original STR is fixed in the latest Nightly.

Hey Fanolian, I don't use MAC, so not sure about other functionality, can you please verify everything else is working as expected?

I think all is well. Thanks for the quick fix.

Flags: needinfo?(Fanolian+BMO)

Comment on attachment 9240420 [details]
Bug 1729395 - Handle message sender going away during message processing r?robwu

Fix for a 93 regression affecting a Mozilla popular extension, low risk in early betas, approved for 93 beta 4, thanks.

Attachment #9240420 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9240543 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Hello,

Verified the fix on the latest Nightly (94.0a1/20210912213037) and Beta (93.0b4/20210912185727) under Windows 10 x64 and Ubuntu 16.04 LTS.

After opening https://example.com, selecting “Always Open This Site in…” from the add-on pop-up and then a container of choice (tried with Personal, Work and Banking to be sure), the tab instantly reopens in the chosen container and "example.com" is added to the list, this confirming the fix.

For further details, see the attached screenshots.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Attached image FMAC Nightly.png
Attached image FMAC Beta.png
Regressions: 1734984
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: