When I try to set up sync in Firefox Metro I can successfully add the code to Firefox on my Mac and get the success screen. But in Firefox Metro the "Connect to Sync" screen with the message "Waiting for other device..." never goes away. If I wait a 10 min and hit Cancel, the button doesn't respond. The only way to get out of this is to open the task manager and quit Firefox Metro.

We've got something wrong with all of our Message Dialogs (that piece of WIndows 8 UI) where they cannot be dismissed. This might be related to that. If so, it's a dupe of bug 840855.

The root cause was an exception thrown by this line, which prevents WeaveGlue._init from running and leaves our sync UI code in a bad state: http://hg.mozilla.org/mozilla-central/file/f2ec16a9feea/browser/metro/base/content/sync.js#l31 This line was broken because of the ".bind(this)" two lines further down. This means that "onReady" is not actually added as the observer; the observer is actually a bound function that calls onReady, and there's no way to remove it. This patch fixes the exception by adding WeaveGlue as the observer instead of a bound function.

Comment on attachment 717356 [details] [diff] [review] patch Review of attachment 717356 [details] [diff] [review]: ----------------------------------------------------------------- r+* This reasoning sounds sensible, the patch looks good, and patch works for me as expected. However, I'm going to double check with the sync owners to make sure this doesn't break a sync assumption. *(I doubt it, but please don't land until we hear back)

talked to the sync group, you're good to go

Comment on attachment 717356 [details] [diff] [review] patch Setting an unowned r? flag as a reminder to myself to wait for a second review.

Comment on attachment 717356 [details] [diff] [review] patch Review of attachment 717356 [details] [diff] [review]: ----------------------------------------------------------------- Hoping I suffice as the second reviewer. ::: browser/metro/base/content/sync.js @@ +25,5 @@ > if (service.ready) { > this._init(); > + } else { > + Services.obs.addObserver(this, "weave:service:ready", false); > + service.ensureLoaded(); I have a gentle preference for early return, rather than else, but nbd.

(Oops, obviously I missed comment 7 when adding comment 8. But thanks for the review, Richard!)

Beautiful! I just found this bug in my metro awesome bar after successfully syncing.