668542 - Cancelling MP unlock will trigger an error and point user to Sync Preferences dialog window

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Description

[Tony Chung [:tchung]]

The use case doesnt make sense here.  If i cancel out of unlocking a Master Password dialog, and then try to force sync, the yellow sync bar appears with a Preferences button, and takes me to the Sync Prefs window.   There's nothing in the sync prefs window indicating to the user what to do next.

Repro:
1) install 6/30/2011 trunk nightly on S-c: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0a1) Gecko/20110627 Firefox/7.0a1
2) enable master password
3) force a sync
4) master password unlock dialog appears.   Cancel out of it
5) Sync throws a yellow bar error with a Preferences button. Log shown is:

1309451037342	Sync.Service	DEBUG	Exception: Login failed: service.master_password_locked No traceback available
1309451037342	Sync.Service	DEBUG	Not syncing: login returned false.
1309451077293	Sync.Service	INFO	Starting sync at 2011-06-30 09:24:37
1309451077294	Sync.Service	DEBUG	In sync: should login.
1309451077295	Sync.Service	INFO	Logging in user iscgg6sijlrazamiygizww4ta2t54mfx
1309451079420	Sync.Service	DEBUG	Fetching passphrase threw [Exception... "User canceled master password entry"  nsresult: "0x80004004 (NS_ERROR_ABORT)"  location: "JS frame :: resource:///components/crypto-SDR.js :: <TOP_LEVEL> :: line 203"  data: no]; assuming master password locked.
1309451079434	Sync.SyncScheduler	DEBUG	Clearing sync triggers.
1309451079435	Sync.SyncScheduler	DEBUG	Couldn't log in: master password is locked.
1309451079435	Sync.SyncScheduler	CONFIG	Starting backoff, next sync at:Thu Jun 30 2011 09:39:39 GMT-0700 (PDT)

6) Verify clicking Prefs button in the yellow error bar takes user to The Sync Prefs window

Expected:
- take the user to unlocking the master password prefs instead (prefs > privacy)

ActuaL:
- takes user to sync prefs window, where there is no instruction on what to do next.

Comment 1

[Philipp von Weitershausen [:philikon]]

Regression from bug 664792 part 2. I know what's wrong, working on a patch.

Comment 2

[Philipp von Weitershausen [:philikon]]

Attachment: v1

Previously Service.login() special-cased MASTER_PASSWORD_LOCKED and didn't notify weave:service:login:error. Bug 664792 part 2 got rid of that because we *want* it to notify login error. We forgot that this would also trigger an error bar, however.

Fortunately, there's a hook that the UI calls before deciding to show an error: Weave.Service.shouldIgnoreError(). So this amends that function to ignore MASTER_PASSWORD_LOCKED, thereby getting us back to the same UI behaviour.

(All of our error handling code makes me sad. It's time we ripped this out of Service, the UI, and various other places, and created a centralized policy object.)

Comment 3

[Richard Newman [:rnewman]]

(In reply to comment #2)

> (All of our error handling code makes me sad. It's time we ripped this out
> of Service, the UI, and various other places, and created a centralized
> policy object.)

Long overdue!

Comment 4

[Philipp von Weitershausen [:philikon]]

http://hg.mozilla.org/services/services-central/rev/51fa361f2ec1

Comment 5

[Philipp von Weitershausen [:philikon]]

http://hg.mozilla.org/mozilla-central/rev/51fa361f2ec1