Closed Bug 679773 Opened 9 years ago Closed 9 years ago

Replace custom helpers with Services.jsm (Port Bug 648364)

Categories

(SeaMonkey :: Sync UI, defect)

defect
Not set

Tracking

(seamonkey2.3 wontfix, seamonkey2.4 fixed, seamonkey2.5 fixed, seamonkey2.6 fixed)

RESOLVED FIXED
seamonkey2.6
Tracking Status
seamonkey2.3 --- wontfix
seamonkey2.4 --- fixed
seamonkey2.5 --- fixed
seamonkey2.6 --- fixed

People

(Reporter: hhofer42, Assigned: InvisibleSmiley)

References

Details

(Keywords: regression)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:8.0a1) Gecko/20110816 Firefox/8.0a1 SeaMonkey/2.5a1
Build ID: 20110816003027

Steps to reproduce:

Trying to connect to an existing sync account or editing sync preferences, manage account, reset account.
On the Sync Options page, select 'Replace all data on this computer...', then press 'Done' or 'Next'


Actual results:

Nothing (on the Sync Options page), but in the error log there's an entry that says:

> Error: Weave.Svc.History is undefined
> Source File: chrome://communicator/content/sync/syncSetup.js
> Line: 778


Expected results:

Confirm replacement of local data, then continue setup.

Trunk (2.5a1) is also affected.
SeaMonkey 2.2 worked OK, Firefox 6.0 still does.
We probably need to port Bug 648364 - Replace custom helpers with Services.jsm
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Sync cannot 'Replace all data on this computer' → Replace custom helpers with Services.jsm (Port Bug 648364)
Duplicate of this bug: 679823
(In reply to Philip Chee from comment #1)
> We probably need to port Bug 648364 - Replace custom helpers with
> Services.jsm

Right,
http://hg.mozilla.org/mozilla-central/rev/a839caa0357b

and Bug 648364 - Replace custom helpers with PlacesUtils.jsm
http://hg.mozilla.org/mozilla-central/rev/aa2c607672e1

I marked them as todo locally but didn't copy/move them to my Sync Todo folder, which is why I missed to address them. Sorry, will take care of the above RSN.
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #554184 - Flags: review?(neil)
OS: Windows XP → All
Hardware: x86 → All
Version: SeaMonkey 2.3 Branch → Trunk
Attachment #554184 - Flags: review?(neil) → review+
Comment on attachment 554184 [details] [diff] [review]
patch [Checkin: Comments 5 and 11]

http://hg.mozilla.org/comm-central/rev/ad67b3c1a764
Attachment #554184 - Attachment description: patch → patch [Checkin: Comment 5]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.6
Comment on attachment 554184 [details] [diff] [review]
patch [Checkin: Comments 5 and 11]

AFAICS this has propagated to both Aurora and Beta already; requesting approval.
Attachment #554184 - Flags: approval-comm-beta?
Attachment #554184 - Flags: approval-comm-aurora?
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #6)
> Comment on attachment 554184 [details] [diff] [review]
> patch [Checkin: Comment 5]
> 
> AFAICS this has propagated to both Aurora and Beta already; requesting
> approval.

Have you tested this patch on either branch? hard for me to gage a risk assessment from a quick glance.
The errors described here and in bug 679823 don't happen anymore.
But I still get this error after sync finishes:

> Error: Weave is not defined
> Source File: chrome://communicator/content/sync/syncSetup.js
> Line: 575
I don't know what the consequences are...

And, yes it would be _really_ good if this get's fixed on beta!
(In reply to Justin Wood (:Callek) from comment #7)
> > AFAICS this has propagated to both Aurora and Beta already; requesting
> > approval.
> 
> Have you tested this patch on either branch? hard for me to gage a risk
> assessment from a quick glance.

There's no need to test this. The corresponding changes (which are 1:1, only different folders: browser vs suite) have already landed on both FF branches, which is what I was trying to say. The changes are related to Core (Toolkit) changes, where some methods have been moved from Weave.Svc.* to Services.* and PlacesUtils.* (refactoring). IOW, instead of calling A.* we have to now call X.* and Y.* because some methods were moved from A to X and Y. Obviously, still accessing A.* for those will result in "undefined" or "null" errors. It's the equivalent of references to non-existent/moved files, with the unfortunate difference that we get no error at compile time.

To please you, I've just created a build from current comm-beta with the patch. Both Change Password and My Sync Key (see bug 679823) worked again.

Note: 2.3 is also affected by this. Sorry I didn't catch it earlier; now it's too late. Will add to the release notes.
Keywords: regression, relnote
(In reply to H. Hofer from comment #8)
> But I still get this error after sync finishes:
> 
> > Error: Weave is not defined
> > Source File: chrome://communicator/content/sync/syncSetup.js
> > Line: 575
> I don't know what the consequences are...

Would be interesting to know whether this happens in FF, too (since AFAIK the code is equal there) but in any case that's a different cause so deserves its own bug.
Attachment #554184 - Flags: approval-comm-beta?
Attachment #554184 - Flags: approval-comm-beta+
Attachment #554184 - Flags: approval-comm-aurora?
Attachment #554184 - Flags: approval-comm-aurora+
Comment on attachment 554184 [details] [diff] [review]
patch [Checkin: Comments 5 and 11]

http://hg.mozilla.org/releases/comm-aurora/rev/c521b24510c9
http://hg.mozilla.org/releases/comm-beta/rev/012db6331856
Attachment #554184 - Attachment description: patch [Checkin: Comment 5] → patch [Checkin: Comments 5 and 11]
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.