1768695 - Add sync/fxa error states to the Firefox View tabs list

Status: RESOLVED FIXED

(Firefox :: Firefox View, enhancement, P1)

Description

[:Gijs (he/him)]

The figma has 2-3 error states:

• Not being able to contact sync to sign in (when you've never completed the setup to show tabs)
• Not being able to contact sync to fetch tabs (when you've previously signed in but we can't contact sync right now)

The second one may or may not show a button (needs checking with UX).

Comment 1

[Sarah Clements [:sclements]]

Let's decide what checks and actions should be used for each of the 4 error states:

1. “We’re having trouble syncing” - per markh, "weave:service:sync:error" is what we want to observe

2. “Turn on syncing to continue” - ?

3. “Check your internet connection” - check for gNetworkLinkService.linkStatusKnown &&gNetworkLinkService.isLinkUpwhentabsSetupFlowManager` is instantiated and observe "network:offline-status-changed" notifications

4. “Your organization has disabled sync” - identity.fxaccounts.enabled is false and Services.prefs.prefIsLocked(identity.fxaccounts.enabled), although technically I think we'd only have to check if the pref is locked per here.

"Turn on syncing to continue" is the big question mark for me, because I initially thought this was replacing this "Turn on tab syncing" state: https://www.figma.com/file/SE4xHgOW84yLiv7vFugm9R/Firefox-View-Stepping-Stone?node-id=1989%3A30079

Comment 2

[:Gijs (he/him)]

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

3. “Check your internet connection” - check for gNetworkLinkService.linkStatusKnown &&gNetworkLinkService.isLinkUpwhentabsSetupFlowManager` is instantiated and observe "network:offline-status-changed" notifications

I could be wrong but I think this should observe something slightly different to catch updates to those gNetworkLinkService props - https://searchfox.org/mozilla-central/rev/23bf1890e07f780ba70e075bc8f46ffb85d1128c/netwerk/base/nsINetworkLinkService.idl#93-99 . I think offline-status-changed would only get updated if the "work offline" state (like in the File menu) changes, which I don't think would happen if e.g. you lose wifi.

Comment 3

[Sarah Clements [:sclements]]

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

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

3. “Check your internet connection” - check for gNetworkLinkService.linkStatusKnown &&gNetworkLinkService.isLinkUpwhentabsSetupFlowManager` is instantiated and observe "network:offline-status-changed" notifications

I could be wrong but I think this should observe something slightly different to catch updates to those gNetworkLinkService props - https://searchfox.org/mozilla-central/rev/23bf1890e07f780ba70e075bc8f46ffb85d1128c/netwerk/base/nsINetworkLinkService.idl#93-99 . I think offline-status-changed would only get updated if the "work offline" state (like in the File menu) changes, which I don't think would happen if e.g. you lose wifi.

The "network:offline-status-changed" does get triggered if I turn wifi off on my mac, so I think this is fine?

Comment 4

[Josh Berman]

Found original documentation - I can confirm that the "Turn on Syncing to continue" error message is redundant and we can use the original screen that is part of the larger FX View set up process to encourage the user to take that step.

https://docs.google.com/presentation/d/1o42c8iY8jOTWjjvCbrbIYz7IDisnfcqxxB_Z_qigRw0/edit#slide=id.g134b03b7189_0_0 (Slide 18 has original error message matrix)
Error message - https://www.figma.com/file/SE4xHgOW84yLiv7vFugm9R/Firefox-View-Stepping-Stone?node-id=7698%3A154673
Original set up message (use this instead of error message) - https://www.figma.com/file/SE4xHgOW84yLiv7vFugm9R/Firefox-View-Stepping-Stone?node-id=1989%3A30079

Comment 5

[Sarah Clements [:sclements]]

Thanks Josh!

Comment 6

[Sarah Clements [:sclements]]

Per a slack thread about this with Ray and Josh, the network offline and sync disabled by admin errors should show first before we go through any setup flow. The "Turn on syncing to continue" prompt stays as the third step/card in the setup flow.

Comment 7

[Ray Fambro]

Thanks Sarah! Just to confirm, the “turn on syncing to continue” will be the first prompt for user that had previously enabled sync, went through the steps, and then later decided to disable it.

For the scenario regarding the sync disabled by admin, we should show the error first.

Comment 9

[Sarah Clements [:sclements]]

Attachment: Bug 1768695 - Add error states handling to FirefoxView r=sfoster

• Add new card and styling for network offline, sync error and sync disabled by admin errors
• Change loading spinner to rotating sync svg

Comment 10

[Sarah Clements [:sclements]]

Attachment: Bug 1768695 - Add error states handling to FirefoxView r=Gijs,sfoster

• Add new card and styling for network offline, sync error and sync disabled by admin errors
• Change loading spinner to rotating sync svg

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8390be8eb18e
Add error states handling to FirefoxView r=sfoster,fluent-reviewers,desktop-theme-reviewers

Comment 12

[Sandor Molnar[:smolnar]]

Backed out changeset 8390be8eb18e (bug 1768695) for causing browser-chrome failures in browser/components/firefoxview/tests/browser/browser_tab_pickup_list.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/5cfb0a8308a8add2af14fd7dd3335d6bcaa09b2f

Push with failures

Failure log

Comment 13

[Sarah Clements [:sclements]]

Hi Sandor, this patch didn't touch the browser_tab_pickup_list.js or the tab_pickup_list.js (and those tests pass locally, on macos at least), so I don't think this is the cause of the failures.

Comment 14

[Sandor Molnar[:smolnar]]

@Sarah, backfills are perma-failing up until your patch
Before that are green

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a9393acb7458
Add error states handling to FirefoxView r=sfoster,fluent-reviewers,desktop-theme-reviewers

Comment 16

[Sandor Molnar[:smolnar]]

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