686366 - Canceling Sign In wizard page triggers "Weave is not defined" error

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch [Checkin: comment 2]

Found this in SM and FF. If we find a better solution together I'll back-port that one to SM instead. :-)

Error: Weave is not defined
Source File: chrome://browser/content/syncSetup.js
Line: 621

STR:
1. Set Up Sync
2. Connect
3. I don't have the device with me
4. Cancel

Reasons:
1. Weave object not available in the context anymore
2. Wrong condition checked for dialog cancel

You know the code better than me, maybe you'll find a reason to keep the old !error check.

Comment 1

[Philipp von Weitershausen [:philikon]]

Comment on attachment 559898 [details] [diff] [review]
patch [Checkin: comment 2]

Thanks for tracking this down! const'ing JPAKE_ERROR_USERABORT outside the onAbort callback makes sense because onAbort might be called if the window context (and hence the 'Weave' object) is already gone, but I think it's a good idea to document in a small comment.

r=me with that. If you have commit privs, please land this on services-central.

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 559898 [details] [diff] [review]
patch [Checkin: comment 2]

http://hg.mozilla.org/services/services-central/rev/eef657744227

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

(In reply to Philipp von Weitershausen [:philikon] from comment #1)
> I think it's a good idea to document in a small comment.

FTR, the comment I added reads "When onAbort is called, Weave may already be gone".

> If you have commit privs, please land this on services-central.

Done. If anything else needs to be done to drive this into mozilla-central, please go ahead since I'm not too involved in that.

Comment 4

[Philipp von Weitershausen [:philikon]]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #3)
> Done. If anything else needs to be done to drive this into mozilla-central,
> please go ahead since I'm not too involved in that.

Thanks for landing! We merge s-c weekly to m-c. RESOLVED FIXED means that it's in m-c, so I'm reopening the bug until that happens.

Comment 5

[Tracy Walker [:tracy]]

following the STR's from the initial report I am not able to reproduce this bug on m-c.  

Is the reported error in the error console?  Is it platform specific?

Comment 6

[Philipp von Weitershausen [:philikon]]

(In reply to Tracy Walker [:tracy] from comment #5)
> Is the reported error in the error console?

Yes.

> Is it platform specific?

Maybe. It may also occur in other cases when manually aborting the setup wizard in the "I already have a Firefox Sync account" branch.

Comment 7

[Tracy Walker [:tracy]]

explored around this and could not turn up the bug on Mac nor Win 7

verified s-c

I hope reporter can verify on nightly tomorrow.

Comment 8

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/mozilla-central/rev/eef657744227

Comment 9

[Jens Hatlak (:InvisibleSmiley)]

(In reply to Tracy Walker [:tracy] from comment #7)
> I hope reporter can verify on nightly tomorrow.

Verified with 2011-09-22 nightly.