Closed Bug 853135 Opened 8 years ago Closed 8 years ago

The Attachments menu belongs under the Message menu (port seamonkey bug 352696)

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 23.0

People

(Reporter: mkmelin, Assigned: mkmelin)

Details

Attachments

(2 files, 1 obsolete file)

Attached patch proposed fix (obsolete) — Splinter Review
It's way illogical for the Attachments menu to be under File. It should of course be under the Message menu. Seamonkey fixed this years ago in bug 352696.

(See bug 352696 discussion, if you wonder why it's a little more changes than one would think.)
Attachment #727340 - Flags: review?(bwinton)
Attachment #727340 - Attachment is obsolete: true
Attachment #727340 - Flags: review?(bwinton)
Attached patch proposed fix, v2Splinter Review
This one.
Attachment #727354 - Flags: review?(bwinton)
Comment on attachment 727354 [details] [diff] [review]
proposed fix, v2

So, this mostly seems good to me, but…

>+++ b/mail/base/content/mailWindowOverlay.xul
>@@ -982,23 +982,16 @@
>         </splitmenu>
>-        <menu id="appmenu_fileAttachmentMenu"
>-              label="&openAttachmentCmd.label;"
>-              disabled="true"
>-              persist="hidden">
>-          <menupopup id="appmenu_attachmentMenuList"
>-                     onpopupshowing="FillAttachmentListPopup(this);"/>
>-        </menu>
>         <menuseparator class="appmenu-menuseparator"/>
>@@ -1920,16 +1913,23 @@
>             <menuseparator id="appmenu_messageAfterOpenMsgSeparator"/>
>+            <menu id="appmenu_msgAttachmentMenu"
>+                  label="&openAttachmentListCmd.label;"
>+                  disabled="true">
>+              <menupopup id="appmenu_attachmentMenuList"
>+                         onpopupshowing="FillAttachmentListPopup(this);"/>
>+            </menu>
>+            <menuseparator id="appmenu_messageAfterAttachmentMenuSeparator"/>

I think that the attachments for a message are important enough to leave in the top level, so let's revert this change.
(Or, well, let's revert part of this change.  I'm happy to change the id from "appmenu_fileAttachmentMenu" to "appmenu_msgAttachmentMenu".  ;)
(Reverting this also makes the two parts of the menu be the same length, which is kind of nice…)

r=me, and ui-r=me with that bit reverted.

Thanks,
Blake.
Attachment #727354 - Flags: ui-review+
Attachment #727354 - Flags: review?(bwinton)
Attachment #727354 - Flags: review+
Attached image appmenu screenshot
> I think that the attachments for a message are important enough to leave in the top level,

Well, for a *message* ;) On the other hand i don't know who (except possibly keyboard only users) would access accachments from the menus, as there are way more natural access points. It's in no way primary.

> Reverting this also makes the two parts of the menu be the same length

The "parts" are still same length, see the attached screenshot. If this was part of the descision, please reconsider. Otherwise, reconsider anyway, as having it in different places creates a bit of inconsistency.
Flags: needinfo?(bwinton)
It's not exactly in different places, since we already hoist a variety of things to that left-hand menu.  I would even venture to say that it'll be less surprising to users, since I'm asking us to not move the attachments out of the left-hand menu, but instead to leave them where they are.  ;)

And the larger gap at the bottom in the screenshot you posted seems uglier to me than having the attachments menu where it used to be.

I understand your objection, and have thought about it, but I stand by my original request.
Flags: needinfo?(bwinton)
http://hg.mozilla.org/comm-central/rev/68687817c22d -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 23.0
You need to log in before you can comment on or make changes to this bug.