Signed in status reported when device name assigned despite fxaccounts being disabled
Categories
(Firefox :: Toolbars and Customization, defect)
Tracking
()
People
(Reporter: ke5trel, Assigned: mconley)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 external file)
STR:
- Launch latest Nightly 156.0a1. with:
identity.fxaccounts.enabled = false
identity.fxaccounts.account.device.name = test - Check browser chrome root html element attributes and open hamburger menu.
Expected:
fxadisabled="true"
fxastatus="not_configured"
"Sign in to sync" appears at the top of hamburger menu (until Bug 2065899 fixed).
Actual:
fxadisabled="true"
fxastatus="signed_in"
"Sign in to sync" does not appear in hamburger menu due to account status reporting as signed in.
Signed in status should be not be possible with accounts disabled. Does not happen with identity.fxaccounts.enabled = true.
This was encountered with Bug 2065899 where FxA disabled state was not being checked but hard to notice on profiles that had device name assigned giving false signed in status.
Regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b2c9624b48f0dc756adcba7cd43a941f349720f6&tochange=46580fa5a4ce20c8cf1c7c9f3f520a1e9c644518
Regressed by Bug 1686521.
Comment 1•2 months ago
|
||
:sfoster, since you are the author of the regressor, bug 1686521, could you take a look? Also, could you set the severity field?
For more information, please visit BugBot documentation.
Comment 2•2 months ago
|
||
Team is discussing and triaging issues related to sign-in UI
Updated•2 months ago
|
Comment 3•2 months ago
|
||
The severity field is not set for this bug.
:Gijs, could you have a look please?
For more information, please visit BugBot documentation.
Comment 4•2 months ago
|
||
(In reply to Stephen Thompson [:sthompson] from comment #2)
Team is discussing and triaging issues related to sign-in UI
What was the outcome of triage / what severity should this get?
(In reply to Kestrel from comment #0)
STR:
- Launch latest Nightly 156.0a1. with:
identity.fxaccounts.enabled = false
identity.fxaccounts.account.device.name = test- Check browser chrome root html element attributes and open hamburger menu.
Expected:
fxadisabled="true"
fxastatus="not_configured"
"Sign in to sync" appears at the top of hamburger menu (until Bug 2065899 fixed).Actual:
fxadisabled="true"
fxastatus="signed_in"
"Sign in to sync" does not appear in hamburger menu due to account status reporting as signed in.
Also, although I appreciate the specific steps to reproduce, can you shed more light on how a profile would end up in this state (with fxa disabled but a device name set anyway) ? That would probably help gauge how many people would be affected / how important this issue is. And, did bug 2065899's fix address this or not?
Comment 5•2 months ago
|
||
Bug 2065899 addressed the display of the sign in button in the app menu. It requires disabling identity.fxaccounts.enabled and then restarting to hide the toolbarbutton and app menu entry.
Starting with identity.fxaccounts.enabled=false and a non-empty identity.fxaccounts.account.device.name does result in fxadisabled="true" fxastatus="signed_in" on the root element. In this state, the toolbarbutton and app menu entry are hidden. I don't see any user-facing issues with the UI's fxastatus="signed_in" value even though it is logically incorrect.
I think there are a couple of related issues:
- toggling
identity.fxaccounts.enabledoff does not seem to trigger FxAccount to start up/tear down - toggling
identity.fxaccounts.enableddoes not trigger UI updates to accounts-related UI
(1) might be good for the Accounts or Sync teams to address. If not signing out, then at least making sure the stored account data is in the expected state after disabling. Signing out happens to remove identity.fxaccounts.account.device.name, for example.
This bug could be expanded to track (2). If fxadisabled and fxastatus can update live, then disabling FxAccounts would immediately hide the UI. Re-enabling FxAccounts would immediately show the UI. A restart wouldn't be required.
I'm assuming that toggling the accounts feature is a rare operation and it's not too much trouble to do a browser restart.
Comment 6•2 months ago
|
||
Maybe there's more context here that isn't written down in this bug or the related bug 2065899, but I'm struggling to see how this is a valid bug report.
"If I set some prefs, then the browser UI looks weird" is not a meaningful set of steps to reproduce and does not actually indicate there's a user-facing bug. There are many ways someone could mess with prefs to put their browser in an incoherent state--that's why Firefox shows a warning screen when a user visits about:config.
With apologies, closing this as invalid. If there is a specific reproducible set of interactions with the browser UI that puts the accounts UI into an inconsistent state, then please reopen the bug with those steps included.
The device name pref was never set by me and existed on multiple profiles on separate devices that I don't recall ever signing in with. Regardless of how it became set, signed in state should never be possible with fxaccounts disabled via Enterprise Policies and will cause user-facing issues down the line.
Comment 8•2 months ago
|
||
| frontend-triage | ||
browser.js sets fxastatus from a startup guess based on identity.fxaccounts.account.device.name, outside the enable && syncEnabled check below it, so a profile with accounts disabled gets fxastatus="signed_in", and browser-sync.js never corrects it because onFxaDisabled() writes only fxadisabled while init() returns before updateAllUI() can run. The guessed string is also one nothing else writes, since updateAllUI uses signedin at #2331, so no rule in panelUI-shared.css matches it.
On comment #4: getLocalName() in FxAccountsDevice.sys.mjs writes a computed default name into that pref the first time it is read, with no signed-in or identity.fxaccounts.enabled check, and BackupService.sys.mjs reads it while building a backup manifest, so a profile that has taken a backup can hold a device name without ever signing in.
Fix: Move the guess inside the syncEnabled branch and write not_configured when accounts are disabled, and have onFxaDisabled() set fxastatus="not_configured" so a window converges when the pref is flipped after startup. Extend test_app_menu_fxa_disabled in browser_sync.js to assert both attributes with a device name set.
Suggested severity: S4. The attribute is logically wrong but has no user-visible effect now that bug 2065899 gated the app menu promo on fxadisabled, and reaching it needs accounts disabled by policy plus a stale device name.
If you'd like to provide feedback on this comment, please use the 👍 or 👎 reaction.
If you want to categorize your feedback you can add one of the following tags: ai-triage-wrong-file, ai-triage-wrong-cause, ai-triage-hallucination, ai-triage-out-of-scope, ai-triage-wrong-fix, ai-triage-shallow-fix.
| Assignee | ||
Comment 9•2 months ago
|
||
Hey Sammy, does that analysis seem right to you? That we should consider adjusting the check for fxastatus to include a check for fxaccounts.enabled state?
Comment 10•1 month ago
|
||
Yes, that analysis looks right. updateFxaToolbarMenu() makes its startup guess before the enable && syncEnabled check, while gSync.init() exits through onFxaDisabled() before updateAllUI() can correct it. I’d gate the device-name guess on identity.fxaccounts.enabled and have onFxaDisabled() set fxastatus="not_configured".
I wouldn’t “fix” signed_in to signedin. signed_in is effectively a neutral startup state: it avoids the not_configured styling without opting into the actual signed-in styling. Changing it to signedin would make a device-name-only profile look signed in. If we rename it, unknown would better describe its purpose.
This state also doesn’t require manually editing prefs: FxAccountsDevice.getLocalName() persists a default device name on first read regardless of FxA state, and BackupService reads it when creating a backup manifest. A profile can therefore have a device name without ever signing in, so I think it was right to reopen this.
Comment 11•1 month ago
|
||
updateFxaToolbarMenu() in browser.js set fxastatus="signed_in" whenever identity.fxaccounts.account.device.name was non-empty, without checking identity.fxaccounts.enabled. A device name can be persisted without ever signing in (getLocalName persists a default on first read; BackupService reads it when building a backup manifest), so a profile with Mozilla accounts disabled (e.g. by enterprise policy) reported fxastatus="signed_in". gSync.init() also returns through onFxaDisabled() before updateAllUI() can correct the attribute.
Gate the startup guess on syncEnabled (identity.fxaccounts.enabled) so a disabled profile stays not_configured, and set fxastatus="not_configured" in onFxaDisabled() so a window still converges when the pref is flipped after startup. The signed_in guess string is intentionally left unchanged (neutral startup state), per comment #10.
Updated•1 month ago
|
Updated•1 month ago
|
Comment 12•1 month ago
|
||
Comment 13•1 month ago
|
||
| bugherder | ||
Comment 14•1 month ago
|
||
The patch landed in nightly and beta is affected.
:mconley, please make an uplift decision for beta:
- Nominate the patch for beta uplift approval if the fix should be included in this release, or
- Set
status-firefox157towontfixif the fix can wait for the next release.
See Requesting an Uplift for documentation on how to request an uplift.
For more information, please visit BugBot documentation.
Comment 15•1 month ago
|
||
Marking wontfix for 157 since we're coming up on the end of beta and this is a long-standing S4
Updated•1 month ago
|
Description
•