Closed
Bug 388206
Opened 17 years ago
Closed 17 years ago
Lightning 0.7pre breaks toolbar customization
Categories
(Calendar :: Lightning Only, defect)
Calendar
Lightning Only
Tracking
(Not tracked)
VERIFIED
FIXED
0.7
People
(Reporter: gordon.burgessparker, Assigned: michael.buettner)
References
Details
(Whiteboard: [patch in hand])
Attachments
(1 file, 1 obsolete file)
17.43 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4 Build Identifier: Installed Lightning 0.7pre on Thunderbird 2.0.0.4. main Toolbar can be customised - if I open a message in a full new window and try to customize that toolbar, when I clck on "Customize" nothing happens - no dialog box appears. if I remove 0.7pre then the customize dialog box returns. Also works OK with 0.5.... Reproducible: Always Steps to Reproduce: 1.Install Lightning 0.7pre 2.Open Message (whether Mail or News) in full window 3.Right-click on Toolbar, choose "Customize". Nothing happens. Actual Results: As above Expected Results: The "Customize" dilog box should have appeared No theme installed.
Assignee | ||
Comment 1•17 years ago
|
||
Does the error console contain any valuable information?
Comment 2•17 years ago
|
||
Confirming on a current Lightning nightly on WinXP. The error console shows: Error: modebox has no properties Source File: chrome://lightning/content/messenger-overlay-toolbar.js pointing to the last line of this code statement: // install the callback that handles what needs to be // done after a toolbar has been customized. mailbox.customizeDone = ModeToolboxCustomizeDone; modebox.customizeDone = ModeToolboxCustomizeDone;
Status: UNCONFIRMED → NEW
Component: General → Lightning Only
Ever confirmed: true
OS: Windows XP → All
QA Contact: general → lightning
Hardware: PC → All
Summary: Lightning 0.7pre breakes toolbar customization → Lightning 0.7pre breaks toolbar customization
Assignee | ||
Comment 3•17 years ago
|
||
In case we're not customizing the standard thunderbird toolbar, the code should not be routed through the modified version but should take the original code path.
Assignee: nobody → michael.buettner
Flags: blocking-calendar0.7+
Assignee | ||
Comment 4•17 years ago
|
||
This patch modifies the customize toolbar dialog to make it aware of the fact that it could have been called from a different context. Lightning overlays the customize toolbar dialog from toolkit to add the drop down list that allows to hop from toolbar to toolbar (those that are available in the main Thunderbird window) and the location option that is specific to the mode toolbar. Besides these obvious bits and pieces are some functions that have been overridden. All of this doesn't apply if the dialog is used to perform the customization for toolbars other than the ones in the main window.
Attachment #277524 -
Flags: review?(philipp)
Assignee | ||
Updated•17 years ago
|
Whiteboard: [patch in hand]
Comment 5•17 years ago
|
||
Michael, will this patch also fix the Composer window (Bug 392940)?
Assignee | ||
Comment 6•17 years ago
|
||
(In reply to comment #5) > Michael, will this patch also fix the Composer window (Bug 392940)? As far as I can see Bug 392940 and Bug 388206 (this one) are duplicates of each other. But yes, this patch allows us to customize the toolbar of the mail composer window.
Comment 7•17 years ago
|
||
Comment on attachment 277524 [details] [diff] [review] patch v1 >-initDialog = function() >-{ >- // remember initialo toolbar location and set the >- // menulist accordingly. this applies to the mode toolbar only. >- gPreviousLocation = gToolbox.getAttribute("location"); >- document.getElementById("loction-list").value = gPreviousLocation; >+initDialog = function() { >+ >+ // Don't do any extra processing in case we're not >+ // customizing one of the main application toolbars. >+ if (gIsMainApplicationContext) { >+ >+ // remember initialo toolbar location and set the >+ // menulist accordingly. this applies to the mode toolbar only. >+ gPreviousLocation = gToolbox.getAttribute("location"); >+ document.getElementById("loction-list").value = gPreviousLocation; Mickey, could you please correct the spelling of "loction-list" in both customize-toolbar.xul and customize-toolbar.js when you're checking in this patch, because this spelling error calls for future bugs.
Comment 8•17 years ago
|
||
* Go to mail mode * Double click on an email * Right click, customize * Error in console: Error: command has no properties Source File: chrome://lightning/content/messenger-overlay-toolbar.js Line: 67 * Go to calendar mode * go back to the open email * right click, customize * Error in console: Error: modebox has no properties Source File: chrome://lightning/content/messenger-overlay-toolbar.js Line: 215 Am I doing something wrong? I deleted xpi-stage/lightning* and did a new build to be sure, but same errors.
Comment 9•17 years ago
|
||
Recognized the bug today in TB 1.5.0.9 and latest nightly. The patch corrects the customization of composer window, but the same errors Philipp describes in comment #8.
Assignee | ||
Comment 10•17 years ago
|
||
Philipp, you didn't do anything wrong, but accessing the customize toolbar dialog from an open mail window is interestingly a bit different than invoking it from a plain composer window. This patch incorporates these additional steps.
Attachment #277524 -
Attachment is obsolete: true
Attachment #278778 -
Flags: review?(philipp)
Attachment #277524 -
Flags: review?(philipp)
Comment 11•17 years ago
|
||
Comment on attachment 278778 [details] [diff] [review] patch v2 Looks good, works as advertised. Only your favorite style nits ;) r=philipp >+ // remember initialo toolbar location and set the typo >+ // menulist accordingly. this applies to the mode toolbar only. >+ gPreviousLocation = gToolbox.getAttribute("location"); >+ document.getElementById("loction-list").value = gPreviousLocation; >+ Trailing whitespaces >+onCancel = function() { >+ >+ // Don't do any extra processing in case we're not >+ // customizing one of the main application toolbars. As an added bonus, you could rather explain why extra processing is needed. >+ if (isModeToolbox) { >+ EnableDisableHierarchy(menubar,true); >+ EnableDisableHierarchy(mailbar,true); >+ EnableDisableHierarchy(calendarbar,true); >+ } else { >+ EnableDisableHierarchy(modebar,true); >+ } Add some spaces after the commas here
Attachment #278778 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 12•17 years ago
|
||
patch checked in on trunk and MOZILLA_1_8_BRANCH -> FIXED
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•17 years ago
|
Target Milestone: --- → 0.7
You need to log in
before you can comment on or make changes to this bug.
Description
•