Closed Bug 1901899 Opened 2 months ago Closed 2 months ago

Fx127 upgrade first run: Dismissing Primary Password prompt prevents session restore

Categories

(Firefox :: General, defect)

Firefox 127
Desktop
All
defect

Tracking

()

VERIFIED FIXED
129 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox127 + verified
firefox128 --- verified
firefox129 --- verified

People

(Reporter: jscher2000, Assigned: Gijs)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(4 files)

This problem occurs for users with a Primary Password set. It is especially acute for users who set Firefox to restore their previous session at startup (but users without that setting also lose access to their previous session).

As noted in bug 1898884, Firefox 127 tries to perform some encryption at first run. If the user has a Primary Password set, Fx127 presents the dialog out-of-context [in front of a blank frame] very early in the startup sequence. Users are accustomed to seeing the dialog after windows have launched. If the user dismisses the dialog [because not immediately relevant], Fx127 starts up in a broken state with no ability to restore the previous session. It would be better/safer if this encryption was deferred until the browser had restored the session.

SUMO thread: https://support.mozilla.org/questions/1449541


Browser Console output related to this sequence of events:

(1) Launch Browser - Primary Password dialog is immediately presented

(2) Cancel Primary Password dialog because not relevant to my life

[Browser Console]

15:51:46.418 NS_ERROR_ABORT: User canceled primary password entry crypto-SDR.sys.mjs:85
encrypt resource://gre/modules/crypto-SDR.sys.mjs:85
setSecurePref resource://gre/modules/shared/FormAutofillUtils.sys.mjs:207
setOSAuthEnabled resource://gre/modules/shared/FormAutofillUtils.sys.mjs:234
_migrateUI resource:///modules/BrowserGlue.sys.mjs:4504
BG__beforeUIStartup resource:///modules/BrowserGlue.sys.mjs:1465
BG_observe resource:///modules/BrowserGlue.sys.mjs:1113

15:51:46.457 NS_ERROR_ABORT: User canceled primary password entry crypto-SDR.sys.mjs:85

15:51:47.078 Key event not available on some keyboard layouts: key=“i” modifiers=“accel,alt,shift” id=“key_browserToolbox” browser.xhtml

(3) Home page load rather than previous sessionstore/SessionStore

(4) Open the History menu (Alt+S) -- session items not available

[Browser Console]

15:51:58.200 Uncaught NS_ERROR_ILLEGAL_VALUE: Window is not tracked SessionStore.sys.mjs:3612
ssi_getClosedTabCountForWindow resource:///modules/sessionstore/SessionStore.sys.mjs:3612
getClosedTabCount resource:///modules/sessionstore/SessionStore.sys.mjs:3652
ss_getClosedTabCount resource:///modules/sessionstore/SessionStore.sys.mjs:362
toggleRecentlyClosedTabs chrome://browser/content/browser-places.js:601
_onPopupShowing chrome://browser/content/browser-places.js:701
PlacesMenu chrome://browser/content/places/browserPlacesViews.js:1971
HistoryMenu chrome://browser/content/browser-places.js:573
onpopupshowing chrome://browser/content/browser.xhtml:1

The Bugbug bot thinks this bug should belong to the 'Firefox::Session Restore' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: General → Session Restore

What happens if the user restarts and that time they do provide their pwd? Is the session lost?

Flags: needinfo?(jscher2000)

(Aside: marking this P1/S1 on filing I expect just means this gets missed by triage - you'd be better off pinging us or release management on slack or matrix, or needinfo'ing, if you think something needs immediate attention.)

Blocks: 1898323
Component: Session Restore → General
OS: Unspecified → All
Hardware: Unspecified → Desktop
Severity: S1 → --
Priority: P1 → --

(In reply to :Gijs (he/him) from comment #3)

What happens if the user restarts and that time they do provide their pwd? Is the session lost?

(1) Scenario 1: "Open previous windows and tabs" enabled (browser.startup.page = 3)

The session is not restored, and Restore Previous Session and the Recently Closed items are grayed out. On disk, previous.js was preserved and can be used for manual file swap recovery if the user learns how in a timely manner.

(2) Scenario 2: "Open previous windows and tabs" not enabled (browser.startup.page = 1)

Restore Previous Session and the Recently Closed items are grayed out. On disk, previous.js was preserved and can be used for manual file swap recovery if the user learns how in a timely manner.

(In reply to :Gijs (he/him) from comment #4)

(Aside: marking this P1/S1 on filing I expect just means this gets missed by triage - you'd be better off pinging us or release management on slack or matrix, or needinfo'ing, if you think something needs immediate attention.)

Thanks for the tip.

Flags: needinfo?(jscher2000)

I think that is what happened to me too. However I did not get to see the dialog and in my settings:

  • browser.startup.page 3
  • services.sync.prefs.sync-seen.browser.startup.page true
  • there are no saved passwords
  • ask to save passwords: unchecked
  • Use a Primary Password: unchecked, unset
  • FF Sync enabled and logged in
  • OS: Archlinux

I got a blank tab I couldn't close (close window worked) after the update instead of the previous session.

A restart of the browser does nothing to fix it. I reverted to 126 for now and everything is fine.

The primary password thing just came up in bug 1891433 - these may end up being dupes. Particularly the message 15:51:58.200 Uncaught NS_ERROR_ILLEGAL_VALUE: Window is not tracked SessionStore.sys.mjs:3612 accounts for the symptoms in that bug.

I'm not aware of any Session Restore changes that would have affected this recently. Is it possible to track down with mozregression?

See Also: → 1891433
Component: General → Session Restore

(In reply to Sam Foster [:sfoster] (he/him) from comment #7)

I'm not aware of any Session Restore changes that would have affected this recently. Is it possible to track down with mozregression?

I think it's collateral damage related to failure of the startup encryption referenced in bug 1898884. However, I didn't dig too deeply.

(In reply to jscher2000 from comment #8)

I think it's collateral damage related to failure of the startup encryption referenced in bug 1898884. However, I didn't dig too deeply.

I'm happy to review a patch. Or write a patch if anyone can identify what is going on.

QA Whiteboard: [qa-regression-triage]

The code that causes exceptions is in migration code that lives in BrowserGlue so I'm going to move this back into Fx::General, as I believe that is the correct place.

Component: Session Restore → General
Summary: Fx127 First Run: Dismissing Primary Password prompt prevents session restore → Fx127 upgrade first run: Dismissing Primary Password prompt prevents session restore

The SUMO thread suggests this is fixed in 128 but based on code inspection I don't understand why it would be. I think it's fixed on Linux in 128, but not on Windows/macOS, because the change in bug 1898323 will check canReauth which is always false on Linux. But on macOS/Windows it'll return true and we'll continue trying to run the same migration.

It would be useful if someone could confirm this. Note that to reproduce either way, you'd need a Firefox profile with a primary password set that was created in 126 or earlier (and hasn't yet run 127 or 128 before).

No longer blocks: 1898323
Regressed by: 1898323

Set release status flags based on info from the regressing bug 1898323

:ssachdev, since you are the author of the regressor, bug 1898323, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Flags: needinfo?(ssachdev)

The bug is marked as tracked for firefox127 (release). However, the bug still isn't assigned.

:pluk, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Flags: needinfo?(pluk)

STR:

  1. Run pip install mozregression to install mozregression if you haven't
  2. Run mozregression --launch 126 --repo mozilla-release --profile C:\Users\sasch\Documents\profile-127-upgrade-primary-password --profile-persistence reuse (make a new empty profile directory and give the path to --profile)
  3. Set the primary password
  4. Enable "Open previous windows and tabs" in about:preferences
  5. Open some tabs
  6. Close Firefox
  7. Run mozregression --launch 127 --repo mozilla-release --profile C:\Users\sasch\Documents\profile-127-upgrade-primary-password
  8. Dismiss the primary password prompt
  9. Confirm that you get a perfectly empty new tab with no text in it at all, and the tab cannot be closed. History is empty too and no restored tabs.
  10. Close it and run mozregression --launch 128.0b2 --repo mozilla-beta --profile C:\Users\sasch\Documents\profile-127-upgrade-primary-password and dismiss the prompt
  11. Confirm the same behavior
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Blocks: 1902324
Flags: needinfo?(ssachdev)
Flags: needinfo?(pluk)
Attachment #9407264 - Flags: approval-mozilla-beta?

beta Uplift Approval Request

  • User impact if declined: broken session restore and/or other parts of firefox for people with primary password
  • Code covered by automated testing: no
  • Fix verified in Nightly: no
  • Needs manual QE test: yes
  • Steps to reproduce for manual QE testing: See https://bugzilla.mozilla.org/show_bug.cgi?id=1901899#c14
  • Risk associated with taking this patch: Low
  • Explanation of risk level: One additional if condition that skips a bunch of migration code when the primary password is set
  • String changes made/needed: No
  • Is Android affected?: no
Flags: qe-verify+
Attachment #9407265 - Flags: approval-mozilla-release?

release Uplift Approval Request

  • User impact if declined: broken session restore and/or other parts of firefox for people with primary password
  • Code covered by automated testing: no
  • Fix verified in Nightly: no
  • Needs manual QE test: yes
  • Steps to reproduce for manual QE testing: https://bugzilla.mozilla.org/show_bug.cgi?id=1901899#c14
  • Risk associated with taking this patch: Low
  • Explanation of risk level: One additional if condition that skips a bunch of migration code when the primary password is set
  • String changes made/needed: No
  • Is Android affected?: no
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8cc3ddf3bc3e
exempt primary password users from startup OS reauth migration r=firefox-desktop-core-reviewers ,credential-management-reviewers,ssachdev
Attachment #9407264 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 129 Branch
QA Whiteboard: [qa-regression-triage] → [qa-regression-triage][qa-triaged]
Attachment #9407265 - Flags: approval-mozilla-release? → approval-mozilla-release+

I've replicated this issue using Firefox 126.0.1 on Windows 10 x64 following the STR from Comment 0.
Verified as fixed on Windows 10 x64, macOS 13, and Ubuntu 22.04, as the issue no longer occurs when updating to the latest Nightly 129.0a1 and Firefox 128.0b3 versions.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-regression-triage][qa-triaged]
Flags: qe-verify+
Duplicate of this bug: 1902963
Flags: qe-verify+
Duplicate of this bug: 1902889

Verified as fixed using the latest Firefox 127.0.1 on Windows 10 x64, macOS 13, and Ubuntu 22.04. Updating the flag accordingly.

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.