836702 - Port | Bug 836120 - Reduce memory overhead of Sync when it isn't configured | to Seamonkey

Status: RESOLVED FIXED

(SeaMonkey :: Sync UI, defect)

Comment 1

[Phoenix]

Attachment: Patch v1

First try, just copy of corresponding Firefox changes

Comment 2

[[e-mail]]

Comment on attachment 708588 [details] [diff] [review]
Patch v1

Eww. That whole init sequence needs a rewrite :-(
(As a dead giveaway, there's even an XXX comment...)

Comment 3

[Phoenix]

I'm not quite understand why not implement small fix, making things better now, in favor of some later complete rewrite, but you're the boss :)

Comment 4

[[e-mail]]

Attachment: Proposed patch

* Uses proper observer notifications and event handling
* Adds checks to see whether Weave has started yet
In the sync pane, we want to do the same thing when sync starts as we do for any other notification, so I just added the start topic to the list of topics that are observed, and manually fake a notification if sync is already started.

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 708861 [details] [diff] [review]
Proposed patch

Sorry I'm not going to get to this any time soon, and I think Ian is a better reviewer for this kind of changes anyway.

While you're at it, please remove the whitespace from syncUI.js line 34/35 since it's in context.

Comment 6

[Jens Hatlak (:InvisibleSmiley)]

I didn't come around to test this, but since this is about initialization, please ensure that the following test cases still work with whatever patch is about to be checked in, both with Sync set up and in an unconfigured state, and with no extra messages on the Error Console compared to now.

- Preferences, esp. Sync pane
- Menus, esp. Go (Tabs from Other Computers) and Tools (Set Up Sync / Sync Now)
- Sync Setup wizard
- MailNews window (customizable toolbar Sync button)
- Browser window (customizable toolbar Sync button)
- Opening and closing extra browser windows.

Comment 7

[[e-mail]]

Pushed comm-central changeset 13c3733f053f.