Closed
Bug 392936
Opened 16 years ago
Closed 16 years ago
Switching 'Mode Toolbar' location ('top' to 'bottom') causes Calendar icon to duplicate
Categories
(Calendar :: Lightning Only, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
0.8
People
(Reporter: wim.bos.be, Assigned: michael.buettner)
References
Details
Attachments
(1 file, 1 obsolete file)
2.87 KB,
patch
|
michael.buettner
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6 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
Comment 1•16 years ago
|
||
Works for me using Lightning 0.7pre (2007082004) with Thunderbird 2.0.0.7pre (20070820) on Windows 2000. Might be a Trunk only issue.
Comment 2•16 years ago
|
||
Confirmed as a TRUNK ONLY issue using Lightning 0.6a1 (2007090806) with Thunderbird 3.0a1pre (2007090904). Using Lightning 0.7pre (2007090903) with Thunderbird 2.0.0.7pre (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
Comment 3•16 years ago
|
||
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 2.0.0.7pre (2007093003) + Lightning 0.7 (2007102304) FAILS in Thunderbird 2.0.0.7pre (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
![]() |
||
Comment 4•16 years ago
|
||
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?
Assignee | ||
Comment 5•16 years ago
|
||
See [1] 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... [1] http://lxr.mozilla.org/mozilla1.8/source/calendar/lightning/content/messenger-overlay-sidebar.js#248
![]() |
||
Comment 6•16 years ago
|
||
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).
Updated•16 years ago
|
Flags: wanted-calendar0.8?
Comment 8•16 years ago
|
||
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-
Comment 9•16 years ago
|
||
Workaround: 1. Customize toolbar. Change toolbar location to 'Top' or 'Bottom' as desired. 2. Restart Thunderbird. 3. Customize toolbar. Select 'Restore Default Set'.
Comment 10•16 years ago
|
||
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)
Assignee | ||
Comment 11•16 years ago
|
||
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+
Assignee | ||
Comment 12•16 years ago
|
||
Previous version of the patch merged with Boris recommendation. Carrying forward r+.
Assignee: nobody → michael.buettner
Attachment #298248 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #298678 -
Flags: review+
Assignee | ||
Comment 13•16 years ago
|
||
patch checked in on trunk and MOZILLA_1_8_BRANCH -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Target Milestone: --- → 0.8
Comment 14•16 years ago
|
||
Checked on linux (branch and trunk build 2008012318), windows (trunk) and Mac (branch) -> task is fixed and verified.
Status: RESOLVED → VERIFIED
Updated•15 years ago
|
Flags: wanted-calendar0.8-
You need to log in
before you can comment on or make changes to this bug.
Description
•