969528 - Sync failure upon FxA authentication displays incorrect UI state

Status: VERIFIED FIXED

(Firefox :: Sync, defect, P2)

Description

[Edwin Wong [:edwong]]

desc: in sync 1.1, sync failures were silent. Current sync1.5 immediately displays a unsigned in UI state.

0. In nightly unlink or disconnect from any signin
1. alter your tokenURL pref so that you force sync to fail, I changed the version to 1.5 -> 1.1
services.sync.tokenServerURI='https://token.services.mozilla.com/1.0/sync/1.1'
2. sign in to fxa
3. look at UI and prefs > sync items

actual: displayed the unsigned state

expected: UI should look like signed in state, or an error should display that sync is not working

Comment 1

[Edwin Wong [:edwong]]

moving to firefox:fxa group

Comment 2

[Edwin Wong [:edwong]]

bumping priority

Comment 4

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0004-Bug-969528-only-show-the-login-needed-UI-state-when-.patch

This requires the patches from bug 977502 to work correctly.  It only shows the "must relogin" state when Sync is in a LOGIN_FAILED_LOGIN_REJECTED state.  This should give us parity with the old sync - the prefs pane doesn't show transient error states.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8383456 [details] [diff] [review]
0004-Bug-969528-only-show-the-login-needed-UI-state-when-.patch

Review of attachment 8383456 [details] [diff] [review]:
-----------------------------------------------------------------

Introducing another explicit error state for that situation sounds like a great idea to me!

Comment 6

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0005-Bug-969528-only-show-the-login-needed-UI-state-when-.patch

Updates both sync.js and browser-fxaccounts.js.  Note there is no need to check readyToAuthenticate as Weave.Status.login will only be LOGIN_FAILED_LOGIN_REJECTED when the auth dance is complete.

Comment 7

[Mark Hammond [:markh] [:mhammond]]

https://hg.mozilla.org/integration/fx-team/rev/377c67fb8c1a

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/377c67fb8c1a

Comment 9

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8386512 [details] [diff] [review]
0005-Bug-969528-only-show-the-login-needed-UI-state-when-.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Fxa Sync
User impact if declined: Incorrect error state handling.
Testing completed (on m-c, etc.): Landed on m-c
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None

Note to sheriffs or whoever lands these: These patches may conflict unless they are landed in the order of: Bug 977502, Bug 967015 and Bug 969528

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/83627088b0c0