Precondition

• The user is connected to sync on desktop and on a mobile device;

Affected versions

• 106.0a1 (2022-09-08)

Tested platforms

• Affected platforms: Ubuntu 22.04, macOS 10.14.6
• Unaffected platforms: windows 10

Steps to reproduce

1. Be in the Firefox view tab;
2. From a connected mobile device, change the account password via Sync;
3. Observe the “Tab pickup” section;

Expected result

• "We’re having trouble syncing/ Firefox can’t reach the syncing service right now./ Try again in a few moments." error message is displayed with "Try again" blue button.;

Actual result

• The “Switch seamlessly between devices” title and “Continue” button is displayed in the “Tab pickup” section;

Additional notes

• Might be related to this issue 1789876

Investigated this a bit today and I'm able to reproduce. It seems changing the password puts us in a state where sync is still connected but the user is logged out (with UIState.get() I'm seeing syncEnabled: true, status: "login_failed"). This case isn't covered by the current sync related exitConditions logic for the error state, so on the update we fall through to "not-signed-in", which explains why we're seeing the login message.

It should be a pretty straightforward fix - just changing those lines as well as making sure we show a "sync-error" when these specific conditions occur, so I'm happy to take this on.

For what it's worth this seems likely to be a pretty rare edge case - as soon as the browser is focused again we get a weave:service:sync:error message which puts the UI in an error state. The UI transition from connected => not signed in => error is only really noticeable if the browser is visible with the firefox view page open. If the browser is hidden/minimized or another tab is active the user is likely to only ever see the error message because that transition happens so quickly.

I have most of a fix ready for the reported issue, but digging into it further this seems like it could be a good opportunity for an enhancement to the firefox view error messages. When we detect that the sync UIState is { syncEnabled: true, status: "login_failed" } we could actually tell the user their account has been disconnected and prompt them to re-login, rather than just showing them a "Try again" button (which isn't going to fix their problem because it won't actually do anything until they enter their new fxa password).

For this case where the password was changed on a different device, there's already sync code in place that detects this state and changes the fxa app menu button to say "Account disconnected" + shows a warning indicator next to it (see screenshot). Basically it should be possible to replicate something like that for firefox view.

Ray I'm curious what you think - I'm assuming this would be a follow up / something we want to file a new bug for (if it seems worth doing).

(In reply to Hanna Jones (:hjones) from comment #2)

Created attachment 9295855 [details]
fxa app menu button after changing account password on a different device

I have most of a fix ready for the reported issue, but digging into it further this seems like it could be a good opportunity for an enhancement to the firefox view error messages. When we detect that the sync UIState is { syncEnabled: true, status: "login_failed" } we could actually tell the user their account has been disconnected and prompt them to re-login, rather than just showing them a "Try again" button (which isn't going to fix their problem because it won't actually do anything until they enter their new fxa password).

Previously when a user disconnects from sync, it would trigger a UIState.ON_UPDATE notification per here: `https://searchfox.org/mozilla-central/source/browser/components/firefoxview/firefox-view-tabs-setup-manager.sys.mjs#275 so I'm not sure what has happened. Firefox View would then show the "Turn on syncing to continue" error state with button (directing them to the settings modal to turn it on). Per the recent meeting, I can take a look at why its not working in bug 1792191 :)

I do agree that we have an opportunity to revisit more potential error message scenarios for follow up work. "Try again" vs "Turn on syncing to continue" vs "Account disconnected" all seem to be serving different edge cases.

Hey Phil -- I'd love to get your expertise on a more appropriate message for this scenario. Let's chat on this, I'll find a slot for us to sync.

Let’s use the disconnected error option ("Turn on syncing to continue..."). I think all of the existing errors options that we have come with different reasons around why they are misleading for this edge case…but I’d like to at least surface something that will get the user to their sync settings for now.

In a follow up release, we can work with Content to get a more appropriate message for this. How does that sound? Is this option feasible to land for 107?

(In reply to Ray Fambro from comment #7)

Let’s use the disconnected error option ("Turn on syncing to continue..."). I think all of the existing errors options that we have come with different reasons around why they are misleading for this edge case…but I’d like to at least surface something that will get the user to their sync settings for now.

In a follow up release, we can work with Content to get a more appropriate message for this. How does that sound? Is this option feasible to land for 107?

The thing is though, they're not actually disconnected from sync as I understand it. And in other situations where a user is not signed in (even though they've completed the sync devices flow), we prompt them with the same screen - "Switch seamlessly between devices". I think even re-reading this bug from QA that perhaps the expected result is incorrect. If the only thing that happened was a user changed a password - but they are still connected to sync - they still need to log in again. This is expected behavior I'd think, its more that we need to tweak the message (as you said, get different copy for a user having completed the sync setup but needing to sign in).

Redirecting users to the about:preferences page to re-enable sync (which also makes them sign in again before they can do that, I believe), even though its already enabled, might actually confuse people more than just asking them to sign in again.

What about keeping the current state - "Switch seamlessly between devices"- but hiding the progress bar if a user has devices connected/has completed the sync set up flow previously in the meantime?

Also, I have a patch for bug 1793754 that will hide the "sync error" if the user is not logged in, since we've determined those are mostly transient errors we shouldn't surface to users.

Thanks for the follow up Sarah! Yes, we ultimately need a new message to address this scenario (re: user changes password, but is still connected to sync). I'm fine with using the first step message instead if we feel it could be less confusing. I believe that Hanna suggested this path as a possible option too. I like the idea of tweaking it to be scenario specific as you indicate. Let's use Step 1 (Switch seamlessly between devices), but I'd like Josh's opinion on whether or not we should hide the progress bar. If we hide it, then the card could become visually awkward with the extra blank space. Maybe this isn't an issue though...this scenario shouldn't be very popular amongst users.

Also, thank you for the patch that hides the sync error!

(In reply to Sarah Clements [:sclements] from comment #8)

The thing is though, they're not actually disconnected from sync as I understand it.

For what it's worth, the FxA team does seem to treat this exact scenario as the user having been "disconnected" based on the messaging that appears in the main application menu (there's an earlier screenshot above). I figured there might be some value in having our treatment of this state align with what the FxA team has the user do, but I understand we also have other options available to us that they might not. Happy to go with whatever seems least confusing.

Redirecting users to the about:preferences page to re-enable sync (which also makes them sign in again before they can do that, I believe), even though its already enabled, might actually confuse people more than just asking them to sign in again.

When they get redirected to about:preferences under these circumstances, there's pretty clear messaging telling the user they need to log in again (see screenshot). Also once they've logged in the user doesn't get redirected back to about:preferences to re-enable sync or anything like that, so it's not like they're going to get confronted by that sync selection dialog and be unsure what to do with it because everything is already enabled.

(In reply to Ray Fambro from comment #7)

Let’s use the disconnected error option ("Turn on syncing to continue..."). I think all of the existing errors options that we have come with different reasons around why they are misleading for this edge case…but I’d like to at least surface something that will get the user to their sync settings for now.

In a follow up release, we can work with Content to get a more appropriate message for this. How does that sound? Is this option feasible to land for 107?

For clarity, this is the path that we are pursuing for now until the updated message is provided.

https://hg.mozilla.org/mozilla-central/rev/eb7f4cfe559e

The patch landed in nightly and beta is affected.
:hjones, 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.

Hello,
We have verified this using Firefox 107.0b1 and FxA stage version, on Ubuntu 22.04 and macOS 12.5.1 . The "Turn on syncing to continue/To grab your tabs, you'll need to allow syncing in Firefox/turn on sync in settings" message error is displayed after changing the password for the used FxA account, from a mobile device.
See the attached screenshot.

Is this the intended behavior?

Yes, this is intended behavior until we get new strings for this specific state (see bug 1794610).

Thanks for responding to this Sarah!

(In reply to Sarah Clements [:sclements] from comment #16)

Yes, this is intended behavior until we get new strings for this specific state (see bug 1794610).

Due to the above comment marking this as verified.