Closed Bug 836702 Opened 7 years ago Closed 7 years ago

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

Categories

(SeaMonkey :: Sync UI, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.18

People

(Reporter: pppx, Assigned: neil)

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch Patch v1 (obsolete) — Splinter Review
First try, just copy of corresponding Firefox changes
Attachment #708588 - Flags: review?(neil)
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...)
Attachment #708588 - Flags: review?(neil) → review-
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 :)
Attached patch Proposed patchSplinter Review
* 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.
Assignee: nobody → neil
Attachment #708588 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #708861 - Flags: review?(jh)
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.
Attachment #708861 - Flags: review?(jh) → review?(iann_bugzilla)
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.
Attachment #708861 - Flags: review?(iann_bugzilla) → review+
Keywords: checkin-needed
Pushed comm-central changeset 13c3733f053f.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
Target Milestone: --- → seamonkey2.18
You need to log in before you can comment on or make changes to this bug.