388206 - Lightning 0.7pre breaks toolbar customization

Status: VERIFIED FIXED

(Calendar :: Lightning Only, defect)

Description

[Gordon Burgess-Parker]

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.

Comment 1

[Michael Büttner (no reviews TFN)]

Does the error console contain any valuable information?

Comment 2

[Simon Paquet [:sipaq]]

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;

Comment 3

[Michael Büttner (no reviews TFN)]

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.

Comment 4

[Michael Büttner (no reviews TFN)]

Attachment: patch v1

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.

Comment 5

[Stefan Sitter [:ssitter]]

Michael, will this patch also fix the Composer window (Bug 392940)?

Comment 6

[Michael Büttner (no reviews TFN)]

(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

[Simon Paquet [:sipaq]]

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

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

* 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

[Sven Giermann]

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.

Comment 10

[Michael Büttner (no reviews TFN)]

Attachment: patch v2

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.

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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

Comment 12

[Michael Büttner (no reviews TFN)]

patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED

Comment 13

[Omar Bajraszewski]

verified with lightning 2007083103