Closed Bug 439291 Opened 13 years ago Closed 10 years ago

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

Categories

(Thunderbird :: Mail Window Front End, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a2

People

(Reporter: dds, Assigned: dds)

Details

Attachments

(4 files, 4 obsolete files)

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.
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Summary: Missing Go - Folder menu command → thunderbird doesn't have the account/folder list under the Go menu
The implementation could be further improved by removing the intermediate Folder menu thus saving an additional keystroke.
The previously submitted patch had incomplete patches for mailWindowOverlay.xul
Attachment #330842 - Attachment is obsolete: true
This is a patch against the files distributed with Thunderbird 3.0.4.  It has been tested and it works correctly.
Attachment #331406 - Attachment is obsolete: true
This is an untested hand-crafted patch against the Thunderbird 3.0.9 source code.
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.
Attached patch Patch against trunk 50d164d40d14 (obsolete) — Splinter Review
Attachment #490473 - Attachment is obsolete: true
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.
Attachment #490619 - Flags: ui-review?(clarkbw)
After selecting the subfolder the menu closes and the folder is selected on the folder selection pane.
(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 on attachment 490619 [details] [diff] [review]
Patch against trunk 50d164d40d14

Looks pretty good to me
Attachment #490619 - Flags: ui-review?(clarkbw) → ui-review+
Marco, do you think this would help?
Attachment #490619 - Flags: review?(mkmelin+mozilla)
Attachment #490619 - Flags: review?(bugzilla)
Comment on attachment 490619 [details] [diff] [review]
Patch against trunk 50d164d40d14

No need to request two reviews one is enough.
Attachment #490619 - Flags: review?(mkmelin+mozilla)
Attachment #490619 - Flags: review?(bwinton)
Attachment #490619 - Flags: review?(bugzilla)
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.
Attachment #490619 - Flags: review?(bwinton) → review+
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.
Keywords: checkin-needed
This modifies the original patch (attachment 490619 [details] [diff] [review]) corrected per :bwinton review.
Attachment #490619 - Attachment is obsolete: true
Attachment #491849 - Flags: review?(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.
Attachment #491849 - Flags: review?(bwinton) → review+
Checked in:
http://hg.mozilla.org/comm-central/rev/ff0495607a53
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee: nobody → dds
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 3.3a2
You need to log in before you can comment on or make changes to this bug.