439291 - thunderbird doesn't have the account/folder list under the Go menu

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, enhancement)

Description

[Diomidis Spinellis]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: 3.0a2pre (2008061403) and older

Thunderbird doesn't have a Go - Folder menu command.  This hinders navigating to a folder without a mouse, and is therefore an accessibility issue.   The underlying functionality (menu-based folder navigation) is already part of the code, as one can see by selecting "Move To" on the context menu of a message from the message list pane.  Such a feature used to be there in SeaMonkey and older Mozilla versions.  The Nostalgy plugin doesn't solve the problem, because it doesn't provide hierarchical navigation. 

Reproducible: Always

Steps to Reproduce:
Select the "Go" menu". 
Actual Results:  
 A "Folder" command is missing.  

Expected Results:  
A "Folder" submenu should appear.  Each element of the "Folder" submenu is the name of a folder, which can again be a submenu, if the folder contains subfolders.  This functionlity exists in SeaMonkey.  The submenu's entries are already available in the "Move To" and "Copy To" message context menu commands.

Comment 1

[Magnus Melin [:mkmelin]]

SeaMonkey implementation was bug 21344 - xref bug 349767. The SM menu looks like this:

 -----------------
 Next
 Previous
 Forward
 Back
 -----------------
 Mail Start Page
 -----------------
 Work Mail >
 News & Blogs >
 news.mozilla.org >
 ------------------ 

Hm, we do have a lot of room under the Go menu. I think I like it.

Comment 2

[Diomidis Spinellis]

Attachment: Patch against version 2.0.0.14 (20080421) that implements the proposed change

The implementation could be further improved by removing the intermediate Folder menu thus saving an additional keystroke.

Comment 3

[Diomidis Spinellis]

Attachment: Patch tested on version 2.0.0.16 (20080708) that implements the proposed change

The previously submitted patch had incomplete patches for mailWindowOverlay.xul

Comment 4

[Diomidis Spinellis]

Attachment: Patch for Thunderbird 3.0.4

This is a patch against the files distributed with Thunderbird 3.0.4.  It has been tested and it works correctly.

Comment 5

[Diomidis Spinellis]

Attachment: Patch for Thunderbird 3.0.9 source code

This is an untested hand-crafted patch against the Thunderbird 3.0.9 source code.

Comment 6

[Ludovic Hirlimann [:Usul]]

Diomidis, thank you very much for the paches. To be accepted in to Thunderbird your patch will need a few things :

1) It needs to be against trunk/tip of the tree (see below)
2) as It touches the UI, you'll need to get ui review on it (providing a screen-shot helps the review in terms of speed)
3) You'll need to get code reviews as well.


You'll find the complete set of rules at https://developer.mozilla.org/en/comm-central .

Following the docs at https://developer.mozilla.org/En/Developer_Guide/Source_Code/Getting_comm-central should let you create a patch against trunk/tip.

Comment 7

[Diomidis Spinellis]

Attachment: Patch against trunk 50d164d40d14

Comment 8

[Diomidis Spinellis]

Attachment: User interface: popup menu

This shows the selection of a folder by pressing ALT-G-O-L-A-S (Go - Folder - Local Folders - AFolder - Subfolder) on a Windows-7 system.

Comment 9

[Diomidis Spinellis]

Attachment: User interface: after the selection

After selecting the subfolder the menu closes and the folder is selected on the folder selection pane.

Comment 10

[Diomidis Spinellis]

(In reply to comment #6)
> Diomidis, thank you very much for the paches. To be accepted in to Thunderbird
> your patch will need a few things :
> 
> 1) It needs to be against trunk/tip of the tree (see below)
Done - see comment 7.

> 2) as It touches the UI, you'll need to get ui review on it (providing a
> screen-shot helps the review in terms of speed)
I added two screen shots - see comments 8 and 9.  I don't know how to arrange a UI menu.

> 3) You'll need to get code reviews as well.
When I first added the patches (comments 4 and 5) I sent email to clarkbw asking for a code review.

Comment 11

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 490619 [details] [diff] [review]
Patch against trunk 50d164d40d14

Looks pretty good to me

Comment 12

[David Bolter [:davidb] (NeedInfo me for attention)]

Marco, do you think this would help?

Comment 13

[Ludovic Hirlimann [:Usul]]

Comment on attachment 490619 [details] [diff] [review]
Patch against trunk 50d164d40d14

No need to request two reviews one is enough.

Comment 14

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 490619 [details] [diff] [review]
Patch against trunk 50d164d40d14

>+++ b/mail/base/content/mailWindowOverlay.xul	Mon Nov 15 10:24:39 2010 +0200
>@@ -1332,16 +1332,30 @@
>           <menuseparator id="goNextSeparator"/>
>+          <menu id="goFolderMenu"
>+		    label="&folderMenu.label;"
>+		    accesskey="&folderMenu.accesskey;"
>+                    oncommand="gFolderTreeView.selectFolder(event.target._folder, true)">

Please don't use tabs, and line the attributes up with the "id" attribute.

>+            <menupopup id="menu_GoFolderPopup"
>+                 type="folder"
>+                 showFileHereLabel="true"
>+                 fileHereLabel="&thisFolder.label;"
>+                 fileHereAccessKey="&thisFolder.accesskey;"
>+                 showRecent="true"
>+                 recentLabel="&contextMoveCopyMsgRecentMenu.label;"
>+                 recentAccessKey="&contextMoveCopyMsgRecentMenu.accesskey;"/>

And these should be lines up with the "id" attribute, too.

But other than that, I like it, so r=me with those nits fixed.

(As a side note, we would usually need tests for this, but I think the folder-widget tests probably do a pretty good job of testing this functionality.  Of course, if you'ld like to learn MozMill, and write some tests for this, I would be more than happy to review them!  :)

Thanks,
Blake.

Comment 15

[David :Bienvenu]

I think we need a correctly formatted patch before adding the checkin-needed flag, just to be on the safe side. But once there's a patch with the nits fixed, please add checkin-needed.

Comment 16

[Diomidis Spinellis]

Attachment: Nits fixed following :bwinton code review

This modifies the original patch (attachment 490619 [details] [diff] [review]) corrected per :bwinton review.

Comment 17

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 491849 [details] [diff] [review]
Nits fixed following :bwinton code review

You didn't actually need to re-ask for a review, cause you only fixed the things I asked you to fix, but r=me anyways. :)

Thanks,
Blake.

Comment 18

[Edmund Wong (:ewong)]

Checked in:
http://hg.mozilla.org/comm-central/rev/ff0495607a53