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

VERIFIED FIXED in mozilla9

Status

Cloud Services
Firefox Sync: UI
--
minor
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Tracking

unspecified
mozilla9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [verified in services])

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
Created attachment 559898 [details] [diff] [review]
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.
Attachment #559898 - Flags: review?(philipp)
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.
Attachment #559898 - Flags: review?(philipp) → review+
(Assignee)

Comment 2

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

http://hg.mozilla.org/services/services-central/rev/eef657744227
Attachment #559898 - Attachment description: patch → patch [Checkin: comment 2]
(Assignee)

Comment 3

7 years ago
(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.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [fixed in services]
Status: REOPENED → ASSIGNED
(Assignee)

Updated

7 years ago
Blocks: 686740
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?
(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.
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.
Whiteboard: [fixed in services] → [verified in services]
https://hg.mozilla.org/mozilla-central/rev/eef657744227
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
(Assignee)

Comment 9

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

Verified with 2011-09-22 nightly.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.