386505 - Cannot switch to Calendar mode using Thunderbird 1.5

Status: VERIFIED FIXED

(Calendar :: Lightning Only, defect)

Description

[Markus Adrario [:Taraman]]

In TB 1.5.0.10 Portable Edition I get the following error on clicking the
calendar-view button:

Fehler: mailToolbar has no properties
Quelldatei: chrome://lightning/content/messenger-overlay-toolbar.js
Zeile: 140

the view is not switching.
Lightning Version: 0.7pre (build 2007070103)

This seems to be after the patch from Bug 371916 has been checked in.

Comment 1

[Stefan Sitter [:ssitter]]

Attachment: rev0 - consider Thunderbird 1.5 mail toolbar ID

Thunderbird 2.0 uses the ID "mail-bar2" while Thunderbird 1.5 uses the ID "mail-bar". The ID was changed with Bug 324194.

Comment 2

[Michael Büttner (no reviews TFN)]

Isn't there any better way than calling getElementById() twice and see which one succeeds? Probably do this once in the load-handler and remembering the right one in a global variable? This is nit-picking, but it would make me feel more comfortable ;-)

Comment 3

[Sven Giermann]

I first found the same problem today with TB 1.5 and then this bug.
Verified the patch, but as mentioned in other bugs, equal code should be considered to be a central function.

So think about a function 'getMailBar()' similiar to 'getViewDeck()'.
This would be perfect to change, whenever a better solution is found.

Comment 4

[Michael Büttner (no reviews TFN)]

(In reply to comment #3)
> So think about a function 'getMailBar()' similiar to 'getViewDeck()'.
> This would be perfect to change, whenever a better solution is found.
That's exactly what I tried to ask for in my comment #2 :-)

Comment 5

[Sven Giermann]

Attachment: rev1 - consider Thunderbird 1.5 mail toolbar ID

...just to push the resolution of this bug.
Otherwise you should consider to change the application requirements for Lightning, which would be sad to exclude TB 1.5!

Comment 6

[Stefan Sitter [:ssitter]]

Comment on attachment 270854 [details] [diff] [review]
rev1 - consider Thunderbird 1.5 mail toolbar ID

This patch replaces one call to getElementById (Tb 2) resp. two calls to getElementById (Tb 1.5) with one function call + two calls to getElementById + passing back the return value (Tb 2 and Tb 1.5). In my opinion this is less efficient and adds more overhead than required for this simple task.

Comment 7

[Michael Büttner (no reviews TFN)]

Comment on attachment 270500 [details] [diff] [review]
rev0 - consider Thunderbird 1.5 mail toolbar ID

canceling review request due to updated version from sven. thanks for initially coming up with a solution.

Comment 8

[Michael Büttner (no reviews TFN)]

Comment on attachment 270854 [details] [diff] [review]
rev1 - consider Thunderbird 1.5 mail toolbar ID

>+function getMailBar() {
>+    var tb2Bar = document.getElementById("mail-bar2");
      if (tb2Bar) {
        return tb2Bar;
      }
>+    return document.getElementById("mail-bar");
>+}
Just a minor performance improvement...

Comment 9

[Michael Büttner (no reviews TFN)]

Comment on attachment 270854 [details] [diff] [review]
rev1 - consider Thunderbird 1.5 mail toolbar ID

>+function getMailBar() {
      return document.getElementById("mail-bar2") ||
             document.getElementById("mail-bar");
>+}
or even better...

Comment 10

[Michael Büttner (no reviews TFN)]

Comment on attachment 270854 [details] [diff] [review]
rev1 - consider Thunderbird 1.5 mail toolbar ID

>+function getMailBar() {
>+    var tb2Bar = document.getElementById("mail-bar2");
>+    var tb1Bar = document.getElementById("mail-bar");
>+    return tb2Bar || tb1Bar;
>+}
Putting the retrieval of the appropriate node from the tree into a separate function makes the code cleaner since it avoids code duplication. Furthermore, counting cycles here strikes me as being premature optimization. These functions are called extremely seldom.

But we could write the body of the function as Stefan originally suggested:
      return document.getElementById("mail-bar2") ||
             document.getElementById("mail-bar");

r=mickey with this issue being addressed.

Comment 11

[Michael Büttner (no reviews TFN)]

Attachment: final patch

patch as it is going to be checked in.

Comment 12

[Michael Büttner (no reviews TFN)]

Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED

Comment 13

[Andreas Treumann]

Verified with lightning build 2007071103, task is fixed.