Firefox Multi-Account Containers's "Always Open This Site in" does not work after bug 1708243 part 2
Categories
(WebExtensions :: General, defect)
Tracking
(Fission Milestone:MVP, firefox-esr78 unaffected, firefox-esr91 unaffected, firefox92 unaffected, firefox93+ verified, firefox94 verified)
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)
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
31.83 KB,
image/png
|
Details | |
30.78 KB,
image/png
|
Details |
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
- Install Firefox Multi-Account Containers (MAC) in a new Profile.
- Open https://example.com
- 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.
(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
Assignee | ||
Comment 3•3 years ago
|
||
Thanks for the report and the quick bisection.
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 5•3 years ago
•
|
||
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
Assignee | ||
Comment 6•3 years ago
|
||
Assignee | ||
Comment 7•3 years ago
|
||
Assignee | ||
Comment 8•3 years ago
|
||
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.
Pushed by tjovanovic@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3bd81b9fb804 Handle message sender going away during message processing r=robwu
Comment 10•3 years ago
|
||
bugherder |
Updated•3 years ago
|
Assignee | ||
Comment 11•3 years ago
|
||
(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
Assignee | ||
Updated•3 years ago
|
Comment 12•3 years ago
|
||
Pushed by tjovanovic@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c8dc9d443c4a Regression test for sendMessage after a popup is closed! r=robwu
Assignee | ||
Comment 13•3 years ago
|
||
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?
Assignee | ||
Comment 14•3 years ago
•
|
||
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
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Comment 15•3 years ago
|
||
bugherder |
Reporter | ||
Comment 16•3 years ago
|
||
(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.
Comment 17•3 years ago
|
||
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.
Updated•3 years ago
|
Comment 18•3 years ago
|
||
bugherder uplift |
Comment 19•3 years ago
|
||
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.
Comment 20•3 years ago
|
||
Comment 21•3 years ago
|
||
Description
•