515643 - Lightning messes up Thunderbird's File->New submenu

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect)

Description

[Thomas Stache]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3
Build Identifier: 

The default menu is just:

|------------
| Message
|------------
| Mail account
| Other account
|------------
| Address Book Contact
|------------

with Lightning it is:

|------------
| Message
| Event
| Task
|------------
|------------
| Mail account
|------------
| Other account
| Calendar
|------------
| Address Book Contact
|------------

The separators are misplaced, and too many to boot. Especially the separator between the new "autoconfig" Mail-Account-Wizard and the old "Other accounts" wizard (for News and RSS) is bad, as it makes it easy to overlook the second item.

Reproducible: Always

Comment 1

[Stefan Sitter [:ssitter]]

Using Lightning 1.0pre (20090910) with Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.4pre) Gecko/20090910 Shredder/3.0b4pre the entries look like:

    Message
    Event
    Task
    -----------
    Folder
    Saved Search
    -----------
    Mail Account
    -----------
    Other Account
    Calendar
    -----------
    Address Book Contact

There are no empty blocks but I agree that Lightning should adopt to the Thunderbird changes. The menu entries were different in the past, at least if I compare Thunderbird 3.0b3 with 3.0b4pre.

Comment 2

[Thomas Stache]

I guess I was in a News account, where New->Folder and New->Saved Search don't make sense and are hidden. With Lightning enabled I see the double separator, though...

Comment 3

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

Unfortunate, but we won't take this for the beta unless its an easy patch and it requires not strings. Let see if we can adopt the changes for the next beta though.

Comment 4

[Matthew Mecca [:mmecca]]

The empty block as shown in comment #1 can also be reproduced when in the smart folders view, if there are multiple email accounts set up and a parent folder is selected. In this case the New->Folder and New->Saved Search are also hidden, leaving the two separators next to each other.

Comment 5

[Matthew Mecca [:mmecca]]

Attachment: [checked in] Fix for New menu separator

Adds observes to hide separator when New->Folder is hidden

Comment 6

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

Comment on attachment 416333 [details] [diff] [review]
[checked in] Fix for New menu separator

Looks good, r=philipp

This one is low risk, I'm taking it for the beta.

Comment 7

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

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/2294f5015d4a>
and comm-1.9.1 revs 59ffbb35c7ee and aa94dfbff210

-> FIXED

Comment 8

[Thomas Stache]

You fixed only one aspect of the "messing up", the occasional adjacent separators.
But, with Lightning there is a separator between Thunderbird's "New->Mail Account" and "New->Other Accounts". Please refer to comment 0 where I stated that these should be together without a separator.

The current structure of the "File->New" method looks like this:
http://mxr.mozilla.org/comm-central/source/mail/base/content/mailWindowOverlay.xul#896

The "insertbefore" property here is wrong:
http://mxr.mozilla.org/comm-central/source/calendar/lightning/content/messenger-overlay-sidebar.xul#117
Looking at the code I now think the whole separator #beforenewAccountMenuItem should go...

As the reporter I have to reopen the bug.

Comment 9

[Matthew Mecca [:mmecca]]

Attachment: [short] Fix for New menu part 2

Removes extra separator before New->Other Accounts...

Comment 10

[Martin Schröder [:mschroeder]]

Comment on attachment 417358 [details] [diff] [review]
[short] Fix for New menu part 2

>     <menuseparator id="afterltnNewTask"
>                    insertbefore="menu_newFolder"
>                    observes="menu_newFolder"/>

Do we need this menuseperator? Thunderbird obviously doesn't have a menuseperator between its messages and folder menuitem because they fall in the same category. Maybe we should add our items (event/task) after the virtual folder one? Suggestions welcome!

Comment 11

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

The second patch doesn't need to be part of beta1, we can live with that for now. Will take care in case we spin rc2 or after the beta.

Comment 12

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

Comment on attachment 417358 [details] [diff] [review]
[short] Fix for New menu part 2

r=philipp

Comment 13

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

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/b0eb2617ad68>

-> FIXED