The "Import Data" modal for existing users is not dismissed after the import flow is finalized
Categories
(Firefox :: Messaging System, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox105 | --- | unaffected |
firefox106 | --- | verified |
firefox107 | --- | verified |
People
(Reporter: mcoman, Assigned: aminomancer)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
(deleted),
image/gif
|
Details | |
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
[Notes]:
- Also, the "migrate_close" telemetry ping is not displayed in the browser console after the Import wizard is closed. (E.G.
TELEMETRY PING (about:welcome): {"experiments":{"task-continuity-sync-after-tab-change-rollout-40":{"branch":"sync-after-tab"},"tcp-rollout-beta-phase-iii-tcp-on-by-default-for-remaining-beta-profiles":{"branch":"control"}},"locale":"fr","version":"106.0","release_channel":"beta","event":"CLICK_BUTTON","event_context":"{\"source\":\"migrate_close\",\"page\":\"spotlight\"}","message_id":"FX_MR_106_UPGRADE_1_UPGRADE_IMPORT_SETTINGS","addon_version":"20220920185943","client_id":"3b147754-c319-411f-99a6-5f4c844642bd","browser_session_id":"c9c94d11-ed96-4c45-8765-7f4f144852aa"})
. - I have not managed to reproduce this issue on macOS 11.7 and macOS 12.5.1.
[Affected versions]:
- Firefox Beta 106.0b2 - Build ID: 20220920185943
- Firefox Nightly 107.0a1 - Build ID: 20220921214338
[Affected Platforms]:
- Windows 11 x64
- Windows 10 x64
- Windows 7 x64
- Linux Mint 20.2
[Prerequisites]:
- Have a Firefox version older than 106 installed.
[Steps to reproduce]:
- Open the browser from the prerequisites.
- Update the browser to the latest version.
- Navigate through the Onboarding flow until the "Import Data" modal is displayed.
- Click the "Import from previous browser" button and complete the import flow.
- Observe the behavior.
[Expected result]:
- The "Import Data" modal is dismissed and the next modal is displayed.
[Actual result]:
- The "Import Data" modal is still displayed.
[Additional Notes]:
- The "Import Data" modal is dismissed and the next modal is displayed if a second import action is performed.
- This issue is not reproducible on the "about:welcome" page.
- Attached is a screen recording of the issue:
Assignee | ||
Comment 1•2 years ago
|
||
So I set a bunch of breakpoints and tested this and it looks like on the first time, we're actually not reaching this await until after the migration wizard window closes. So it doesn't start waiting until after the thing it's waiting for... And then if you click the button again and go through the wizard again, the promise resolves as normal.
Why is this happening? I guess this bug must be a pretty low level issue, because once we hit this call, execution stops completely until the migration wizard closes.
It's hard to test this because I can't update from 105 to 106 in a local build, and this isn't the most elegant solution, but I think we can circumvent the issue by changing this:
AboutWelcomeUtils.handleUserAction(action);
// Wait until migration closes to complete the action
if (action.type === "SHOW_MIGRATION_WIZARD") {
await window.AWWaitForMigrationClose();
AboutWelcomeUtils.sendActionTelemetry(props.messageId, "migrate_close");
}
to this:
if (action.type === "SHOW_MIGRATION_WIZARD") {
// Wait until migration closes to complete the action
let migrationClosed = window.AWWaitForMigrationClose();
AboutWelcomeUtils.handleUserAction(action);
await migrationClosed;
AboutWelcomeUtils.sendActionTelemetry(props.messageId, "migrate_close");
} else {
AboutWelcomeUtils.handleUserAction(action);
}
However, it would be good to fix the underlying issue where opening a dialog seems to pause execution in the opener... how does that work? Is the event loop in the opener just spinning until the dialog closes? Why does this only happen after an update?
Assignee | ||
Comment 2•2 years ago
|
||
Ed and Gijs - have you seen anything like this? Execution seems to be frozen in the spotlight dialog content context after the JSM context calls Services.ww.openWindow
Comment 3•2 years ago
|
||
Import wizard is a "true" modal on windows and linux, so it stops javascript execution from the opener. The behavior is different for about:welcome because it uses parent/child message passing whereas upgrade spotlight directly opens the wizard.
Updated•2 years ago
|
Comment 4•2 years ago
|
||
We can probably defer the showMigrationWizard
call for our actions as other direct callers might want the synchronous behavior:
https://searchfox.org/mozilla-central/rev/cefb4bc274c71aa4badbec09da9c056fd8ca4472/toolkit/components/messaging-system/lib/SpecialMessageActions.jsm#228-232
Comment 5•2 years ago
|
||
We might need to update these tests when making the action call from a different event loop
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 6•2 years ago
|
||
QA test plan:
- Enter this in browser console:
Services.prefs.setBoolPref("browser.shell.checkDefaultBrowser", true);
ShellService.isDefaultBrowser = () => false;
Cc["@mozilla.org/browser/browserglue;1"].getService().wrappedJSObject._showUpgradeDialog();
- Skip through the upgrade dialog until the import screen
- Click the "Import from previous browser" button
- Click through the migration wizard, leaving default options
- Click "Finish"
- When bug is fixed, the upgrade dialog should advance to the next screen
Assignee | ||
Comment 7•2 years ago
|
||
Also update tests that individually accounted for this bug.
Updated•2 years ago
|
Comment 9•2 years ago
|
||
bugherder |
Comment 10•2 years ago
|
||
The patch landed in nightly and beta is affected.
:aminomancer, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- If no, please set
status-firefox106
towontfix
.
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 11•2 years ago
|
||
Comment on attachment 9295827 [details]
Bug 1792014 - Fix migration wizard blocking execution in the Spotlight dialog. r=Mardak!
Beta/Release Uplift Approval Request
- User impact if declined: Users will not get any feedback that their initial profile migration worked.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: 1. Enter this in browser console:
Services.prefs.setBoolPref("browser.shell.checkDefaultBrowser", true);
ShellService.isDefaultBrowser = () => false;
Cc["@mozilla.org/browser/browserglue;1"].getService().wrappedJSObject._showUpgradeDialog();
- Skip through the upgrade dialog until the import screen
- Click the "Import from previous browser" button
- Click through the migration wizard, leaving default options
- Click "Finish"
- The upgrade dialog should advance to the next screen
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The changes are very minimal, only wrapping the existing migration wizard invocation in
Services.tm.dispatchToMainThread(...)
to stop it from blocking the scripts in the Spotlight dialog. The affected code was last modified in 103. - String changes made/needed:
- Is Android affected?: No
Assignee | ||
Updated•2 years ago
|
Comment 12•2 years ago
|
||
Approved for 106.0b4, thanks.
Updated•2 years ago
|
Comment 13•2 years ago
|
||
bugherder uplift |
Updated•2 years ago
|
Reporter | ||
Comment 14•2 years ago
|
||
I have verified that this issue is no longer reproducible with the latest Firefox Nightly (107.0a1 Build ID - 20220925213821) and latest Firefox Beta (106.0b4 Build ID - 20220925185751) installed on Windows 10 x64 and Ubuntu 22.04 x64. Now, I can confirm that the "Import Data" modal is dismissed after completing or canceling the import flow.
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Description
•