Closed Bug 1792014 Opened 2 years ago Closed 2 years ago

The "Import Data" modal for existing users is not dismissed after the import flow is finalized

Categories

(Firefox :: Messaging System, defect, P1)

Desktop
All
defect

Tracking

()

VERIFIED FIXED
107 Branch
Iteration:
107.1 - Sept 19 - Sept 30
Tracking Status
firefox105 --- unaffected
firefox106 --- verified
firefox107 --- verified

People

(Reporter: mcoman, Assigned: aminomancer)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached image rec of the issue.gif (deleted) —

[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]:

  1. Open the browser from the prerequisites.
  2. Update the browser to the latest version.
  3. Navigate through the Onboarding flow until the "Import Data" modal is displayed.
  4. Click the "Import from previous browser" button and complete the import flow.
  5. 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:
Priority: -- → P1

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?

Component: Messaging System → Migration

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

Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(edilee)

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.

Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(edilee)
Component: Migration → Messaging System

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

Assignee: nobody → shughes
Status: NEW → ASSIGNED

QA test plan:

  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();
  1. Skip through the upgrade dialog until the import screen
  2. Click the "Import from previous browser" button
  3. Click through the migration wizard, leaving default options
  4. Click "Finish"
  5. When bug is fixed, the upgrade dialog should advance to the next screen

Also update tests that individually accounted for this bug.

Attachment #9295827 - Attachment description: Bug 1792014 - Fix migration wizard blocking the Spotlight content thread. r=Mardak! → Bug 1792014 - Fix migration wizard blocking execution in the Spotlight dialog. r=Mardak!
Pushed by shughes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bed6b311978a Fix migration wizard blocking execution in the Spotlight dialog. r=Mardak
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 107 Branch

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 to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(shughes)

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();
  1. Skip through the upgrade dialog until the import screen
  2. Click the "Import from previous browser" button
  3. Click through the migration wizard, leaving default options
  4. Click "Finish"
  5. 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
Flags: needinfo?(shughes)
Attachment #9295827 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Approved for 106.0b4, thanks.

Attachment #9295827 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

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.

Status: RESOLVED → VERIFIED
Regressions: 1792096
Iteration: --- → 107.1 - Sept 19 - Sept 30
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: