Closed Bug 392936 Opened 17 years ago Closed 16 years ago
Switching 'Mode Toolbar' location ('top' to 'bottom') causes Calendar icon to duplicate
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:126.96.36.199) Gecko/20070725 Firefox/188.8.131.52 Build Identifier: 0.6a1 2007082004 trunk Upon each update of the 'Mode Toolbar' location (top or bottom), the Calendar icon(s) get(s) duplicated. Reproducible: Always Steps to Reproduce: 1. Open 'Mode Toolbar' editor 2. Change 'Location' 3. Change 'Location' again, and again, ... Actual Results: Need to manually remove duplicate icons Expected Results: No need to remove duplicate icons
Works for me using Lightning 0.7pre (2007082004) with Thunderbird 184.108.40.206pre (20070820) on Windows 2000. Might be a Trunk only issue.
Confirmed as a TRUNK ONLY issue using Lightning 0.6a1 (2007090806) with Thunderbird 3.0a1pre (2007090904). Using Lightning 0.7pre (2007090903) with Thunderbird 220.127.116.11pre (20070909) everything is OK.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → Windows 2000
Summary: Switching 'Mode Toolbar' location ('top' to 'bottom') causes Calendar icon to duplicate → [Trunk] Switching 'Mode Toolbar' location ('top' to 'bottom') causes Calendar icon to duplicate
Version: unspecified → Trunk
In contrast to my test results from Comment #2 I now see the issue on MOZILLA_1_8_BRANCH too. I found the following regession range: WORKS in Thunderbird 18.104.22.168pre (2007093003) + Lightning 0.7 (2007102304) FAILS in Thunderbird 22.214.171.124pre (2007100104) + Lightning 0.7 (2007102304) Checkins to MOZILLA_1_8_BRANCH during regression range: <http://tinyurl.com/yuefen> Most probably also regressed by Bug 267833.
Summary: [Trunk] Switching 'Mode Toolbar' location ('top' to 'bottom') causes Calendar icon to duplicate → Switching 'Mode Toolbar' location ('top' to 'bottom') causes Calendar icon to duplicate
Version: Trunk → unspecified
Can someone give me something resembling a minimal testcase that doesn't involve the entire Calendar UI? Or some pointers to the relevant code, at least?
See  for the relevant source code location that handles this particular toolbar. The reason for this rather elaborate handling of the toolbar is that it should be possible to dynamically place this element. Unfortunately, the constructor of the toolbar binding gets called each time the DOM node gets bound to a different parent. That's why I'm touching the internals of the toolbar to get around this problem. It would be great if there's a cleaner solution to the problem...  http://lxr.mozilla.org/mozilla1.8/source/calendar/lightning/content/messenger-overlay-sidebar.js#248
Oh, I see. You're modifying the DOM from inside a mutation listener (a no-no if you want sane behavior, for what it's worth). That means that the constructor won't fire until after your return from the mutation listener and the end of the attribute mutation update. Presumably that's screwing up your save/restore stuff (as in, you restore too early)? If it's reasonable to do so, I'd suggest putting most of the code in the mutation listener off on a timeout (except for the "remove the listener" part, pretty much).
This bug is not important enough to warrant wanted0.8 status. We would however take a fix for this bug into 0.8, if such a fix materializes.
Flags: wanted-calendar0.8? → wanted-calendar0.8-
Workaround: 1. Customize toolbar. Change toolbar location to 'Top' or 'Bottom' as desired. 2. Restart Thunderbird. 3. Customize toolbar. Select 'Restore Default Set'.
Don't know exactly why, but after observing WHEN the duplication occurs, this patch works for me in MOZILLA_1_8_BRANCH (Windows). Can anyone please verify this in TRUNC and maybe on other platforms?
Attachment #298248 - Flags: review?(michael.buettner)
Comment on attachment 298248 [details] [diff] [review] Patch v1 Admittedly, I don't understand why this solves the duplication problem. It is necessary it hold a reference to the toolbox palette, then rearrange the dom tree and reassign the reference to the palette. If we don't manually reassign the palette reference, it magically disappears from the tree. I decided to reassign the palette after the tree has been modified. I don't really understand where's the relation to the presumably unrelated other actions in this function. But as it solves this annoying problem I tend to just take the patch for now and investigate the underlying problem later. On top of that, I would like to follow Boris advice and defer the action until the mutation listener has been finished. I'm going to change the patch accordingly before checking it in. Sven, thanks for the patch. r=mickey.
Attachment #298248 - Flags: review?(michael.buettner) → review+
Previous version of the patch merged with Boris recommendation. Carrying forward r+.
patch checked in on trunk and MOZILLA_1_8_BRANCH -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Checked on linux (branch and trunk build 2008012318), windows (trunk) and Mac (branch) -> task is fixed and verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.