Closed Bug 840643 Opened 12 years ago Closed 12 years ago

Work - Connect to Sync shows waiting for other device forever

Categories

(Firefox for Metro Graveyard :: General, defect)

21 Branch
All
Windows 8.1
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 22

People

(Reporter: verdi, Assigned: mbrubeck)

References

Details

(Whiteboard: feature=work)

Attachments

(2 files)

Attached image Waiting for other device screen (deleted) —
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.
Blocks: 831615
No longer blocks: metrov1triage
Summary: Connect to Sync shows waiting for other device forever → Work - Connect to Sync shows waiting for other device forever
Whiteboard: feature=work
Attached patch patch (deleted) — Splinter Review
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.
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
Attachment #717356 - Flags: review?(ally)
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)
Attachment #717356 - Flags: review?(ally) → review+
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.
Attachment #717356 - Flags: 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.
Attachment #717356 - Flags: review? → review+
(Oops, obviously I missed comment 7 when adding comment 8. But thanks for the review, Richard!)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Beautiful! I just found this bug in my metro awesome bar after successfully syncing.
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: