Last Comment Bug 679773 - Replace custom helpers with Services.jsm (Port Bug 648364)
: Replace custom helpers with Services.jsm (Port Bug 648364)
Status: RESOLVED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: Sync UI (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.6
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
Mentors:
: 679823 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-17 10:18 PDT by H. Hofer
Modified: 2011-09-25 16:08 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
wontfix
fixed
fixed
fixed


Attachments
patch [Checkin: Comments 5 and 11] (5.28 KB, patch)
2011-08-18 13:06 PDT, Jens Hatlak (:InvisibleSmiley)
neil: review+
bugspam.Callek: approval‑comm‑aurora+
bugspam.Callek: approval‑comm‑beta+
Details | Diff | Review

Description H. Hofer 2011-08-17 10:18:28 PDT
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.
Comment 1 Philip Chee 2011-08-18 10:52:23 PDT
We probably need to port Bug 648364 - Replace custom helpers with Services.jsm
Comment 2 Philip Chee 2011-08-18 10:53:55 PDT
*** Bug 679823 has been marked as a duplicate of this bug. ***
Comment 3 Jens Hatlak (:InvisibleSmiley) 2011-08-18 11:16:28 PDT
(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.
Comment 4 Jens Hatlak (:InvisibleSmiley) 2011-08-18 13:06:36 PDT
Created attachment 554184 [details] [diff] [review]
patch [Checkin: Comments 5 and 11]
Comment 5 Jens Hatlak (:InvisibleSmiley) 2011-08-18 23:44:42 PDT
Comment on attachment 554184 [details] [diff] [review]
patch [Checkin: Comments 5 and 11]

http://hg.mozilla.org/comm-central/rev/ad67b3c1a764
Comment 6 Jens Hatlak (:InvisibleSmiley) 2011-08-18 23:54:59 PDT
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.
Comment 7 Justin Wood (:Callek) 2011-08-19 20:00:45 PDT
(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.
Comment 8 H. Hofer 2011-08-20 00:40:16 PDT
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!
Comment 9 Jens Hatlak (:InvisibleSmiley) 2011-08-20 11:41:09 PDT
(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.
Comment 10 Jens Hatlak (:InvisibleSmiley) 2011-08-20 11:42:46 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.