440700 - [Trunk] Cannot switch back from Calendar to Mail Mode [Error: mailToolbarMenuItem is null]

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect)

Description

[Helmut Leininger]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: Lightning 0.6a1build 2008061920

Installation:
Thunderbird Version 3.0a2pre (2008061903)
nightly builds of lightning and gdata-provider dated 20080619

After starting Thunderbird, everything looks ok. Switch tzo the calendar looks ok, too.
When trying to switch back to Thunderbird, the left column (calendar selections ...) is cleared, everything else stays as is. Thunderbird mails and taskbars do not appear.


Reproducible: Always

Steps to Reproduce:
1. Start Thunderbird
2. Click on the Calendar icon (bottom, second from left)
3. Click on the Mail icon (bottom, first left)
Actual Results:  
as described above. Thunderbird must be restarted to get the mail window back.


Expected Results:  
Switching back to mails

After start of Thunderbird, the error console shows the following error (don't know if it has to do something with the problem because everything else seems to work):
Fehler: a._updateVisibleText is not a function
Quelldatei: chrome://global/content/bindings/textbox.xml
Zeile: 208


After switching to the calendar and trying to switch back, there is an additional error message:
Fehler: mailToolbarMenuItem is null
Quelldatei: chrome://lightning/content/messenger-overlay-toolbar.js
Zeile: 105


Terminating Thunderbird afterwards adds these errors:
Fehler: Components is not defined
Quelldatei: chrome://calendar/content/calendar-minimonth-busy.js
Zeile: 66

Fehler: Components is not defined
Quelldatei: chrome://calendar/content/calendar-minimonth-busy.js
Zeile: 66

Fehler: Components is not defined
Quelldatei: chrome://calendar/content/calendar-statusbar.js
Zeile: 128

Fehler: Components is not defined
Quelldatei: chrome://calendar/content/calendar-statusbar.js
Zeile: 128

Fehler: Components is not defined
Quelldatei: chrome://calendar/content/calendar-statusbar.js
Zeile: 128

Comment 1

[Stefan Sitter [:ssitter]]

> Fehler: a._updateVisibleText is not a function

Bug 402573, but I think it's not relevant here.

> Fehler: mailToolbarMenuItem is null

Probably the cause for the failure.

Comment 2

[Stefan Sitter [:ssitter]]

Lightning tries to get the element "menu_showMessengerToolbar" and fails. Attachment 325291 [details] [diff] in Bug 363461 removed or replaced "menu_showMessengerToolbar".

Comment 4

[Mark Banner (:standard8)]

(In reply to comment #2)
> Lightning tries to get the element "menu_showMessengerToolbar" and fails.
> Attachment 325291 [details] [diff] in Bug 363461 removed or replaced
> "menu_showMessengerToolbar".
> 
So MailNews is now dynamically generating the menu, but it has no ids. So I think what we need to do would be to provide ids when we generate the menu in MailNews based on the toolbar id, then lightning can pick up the appropriate id.

Does that sound right?

Comment 8

[Magnus Melin [:mkmelin]]

Not sure if that would be correct or not, but the new thunderbird code is very similar to the firefox one - http://mxr.mozilla.org/seamonkey/source/browser/base/content/browser.js#4474

... and since they don't need to add an id, it sounds a bit wrong.

Just as a note, the code fails at
    var mailToolbar = getMailBar();
    var mailToolbarMenuItem = document.getElementById("menu_showMessengerToolbar");
    if (mailToolbarMenuItem.getAttribute("checked") == "true") {
        mailToolbar.removeAttribute("collapsed");
    }

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: picks the id to ask for based on isBranch(), v1

Comment 10

[James Rome]

When will this fix be in the nightly builds?

Does it also fix the problem of the calendar not being able to do things like switch months in TB3, whereas this works in TB2?

Comment 11

[Stefan Sitter [:ssitter]]

I don't think we should check for the branch, e.g. SeaMonkey still seems to use "menu_showMessengerToolbar". Maybe use a helper function similar to getMailBar()? There also seems to be another place where Lightning reads menu_showMessengerToolbar.
http://mxr.mozilla.org/mozilla/search?string=menu_showMessengerToolbar

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: try both possible ids

A reasonable suggestion.  Here's a better patch.

Comment 13

[Dan Mosedale (:dmosedale, :dmose)]

James: the night after it gets checked in.  Month-switching is unrelated to this, but if you file a new bug for that, I'll try and come up with a patch.

Comment 14

[James Rome]

I did https://bugzilla.mozilla.org/show_bug.cgi?id=442520

Comment 15

[Stefan Sitter [:ssitter]]

The patch doesn't seem correct. Previously Lightning tried to get access to the menu entry "View -> Toolsbars -> Mail Toolbar". That entry no longer has an ID and can't be accessed. 

The patch redirects Lightning to access the menu entry "View -> Status Bar" instead. This might prevent the failure and allow switching back to mail mode but it certainly doesn't reflect the desired behavior.

Comment 16

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 328936 [details] [diff] [review]
try both possible ids

There are other problems with that patch too.

Comment 17

[Stefan Sitter [:ssitter]]

Attachment: add check

Due to the missing ID it's currently not possible to hide the menu entry 'Mail Toolbar' (in messenger-overlay-sidebar.js) when switching to Calendar or Task Mode. Therefore it doesn't matter if we can't unhide it (in messenger-overlay-toolbar.js) when switching back to Mail Mode.

Comment 18

[Stefan Sitter [:ssitter]]

Comment on attachment 330246 [details] [diff] [review]
add check

Bah. This solution has the drawback that the Mail Toolbar is never shown after switching back to Mail Mode.

Magnus, would it be possible to add the ID back to the menu entry so that Lightning can get access to it? If not, is there another way to get the information if the Mail Toolbar should be shown?

Comment 19

[Magnus Melin [:mkmelin]]

What do you do to hide it in the first place?
Would it work to just use the first toolbar (with type != "menubar") of mail-toolbox instead of one with id? (Or whatever's needed to find it.)

Comment 20

[Dan Mosedale (:dmosedale, :dmose)]

Magnus, searching for first toolbar seems notably hackier than just adding an ID.  Really, I'd assert that almost everything in chrome should have IDs specifically for use by extension authors in cases just such as this.  Do you disagree?

Comment 21

[Magnus Melin [:mkmelin]]

No, generally ids are very useful of course. In this case, I just wonder if it wouldn't cause problems, as it's used dynamically so you'd potentially have the same id two times. (One menuitem in the context menu, and the other with the same in the view menu.) Just speculation...

I'd also like to know why hiding in the first place works. Or does it?

Comment 22

[Dan Mosedale (:dmosedale, :dmose)]

Seems like the ideal solution here would be if nsIControllers had, in addition to an "enabled" state, a "hidden" state.  That said, in our particular case, we could just clone the element and insert it with different ids in the different places, if having identical ids causes a problem.

Comment 23

[Stefan Sitter [:ssitter]]

(In reply to comment #19)
> What do you do to hide it in the first place?

Lightning customizes the toolbar behavior in Thunderbird. 

When switching Thunderbird to calendar mode it shows the calendar toolbar and hides the mail toolbar. When switching back to mail mode it shows the mail toolbar and hides the calendar toolbar. 

During the switches it checks the corresponding menu entry in toolbar menu to get the information if the user disabled one of the toolbars manually. In that case it won't be shown automatically. The same feature applies when switching from/to task mode. 

Currently the check for the mail toolbar menu item fails causing the issues. I'm sure the main calendar developers can provide you more technical information if requested.

Comment 24

[Stefan Sitter [:ssitter]]

Attachment: add check v2

This workaround makes it possible to switch back to mail mode in Thunderbird Trunk builds. The drawback is that it will always restore the mail toolbar ignoring the users decision. I suggest to file a follow up bug on this issue.

Comment 25

[Veritas]

I just started experiencing this issue a few days ago -- So it either had been fixed and reappeared, or something else is going on.  Anyone else experiencing it with the latest nightly and latest build of calendar?

Comment 26

[Adam]

It's been over a week now since the nightlies for Lightning aren't compatible with the nightlies for Thunderbird, and the Nightly Tester Tools add-on is incompatible also. Up until then, I'd been testing this issue daily, but it never worked since it was reported. I edited the install.rdf this morning to force the latest Lightning nightly to install on the latest Thunderbird nightly, but it's still having the same problem.

Comment 27

[James Rome]

worse, the Lightning "nightly" builds for Thunderbird are version 0.61 which will trash your 0.8 appointments. So I had to dump TB3 for now. Why can't they at least build the new version?

Comment 28

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

Comment on attachment 330967 [details] [diff] [review]
add check v2

Looks good, r=philipp

Comment 29

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

Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED

Comment 30

[Veritas]

this build IS NOT compatible with the trunk.  why is this not being updated for the trunk build?

Comment 31

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

Vertias, this is bug 448753 and its dependant bug.