Closed Bug 969528 Opened 6 years ago Closed 6 years ago

Sync failure upon FxA authentication displays incorrect UI state

Categories

(Firefox :: Sync, defect, P2, critical)

Product:
Firefox
Component:
Sync
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 29
Tracking Flags:
Tracking Status
firefox29 + verified
firefox30 --- verified

People

(Reporter: edwong, Assigned: markh)

References

Details

(Whiteboard: [qa!])

Attachments

(1 file, 1 obsolete file)

0005-Bug-969528-only-show-the-login-needed-UI-state-when-.patch
3.44 KB, patch
ttaubert
: review+
Sylvestre
: approval-mozilla-aurora+
Details | Diff | Splinter Review 
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
moving to firefox:fxa group
Component: Firefox Sync: UI → FxA
Product: Mozilla Services → Firefox
Target Milestone: --- → Firefox 29
Version: unspecified → 29 Branch
bumping priority
Severity: normal → critical
Priority: -- → P2
Version: 29 Branch → Trunk
Blocks: 969593
No longer blocks: 969593
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 968439
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Assignee: nobody → mhammond
Blocks: 969593
Depends on: 977502
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.
Attachment #8383456 - Flags: feedback?(ttaubert)
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!
Attachment #8383456 - Flags: feedback?(ttaubert) → feedback+
Whiteboard: [qa+]
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.
Attachment #8383456 - Attachment is obsolete: true
Attachment #8386512 - Flags: review?(ttaubert)
Attachment #8386512 - Flags: review?(ttaubert) → review+
https://hg.mozilla.org/mozilla-central/rev/377c67fb8c1a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
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
Attachment #8386512 - Flags: approval-mozilla-aurora?
Attachment #8386512 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: RESOLVED → VERIFIED
status-firefox29: fixed → verified
status-firefox30: fixed → verified
Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.