Last Comment Bug 853135 - The Attachments menu belongs under the Message menu (port seamonkey bug 352696)
: The Attachments menu belongs under the Message menu (port seamonkey bug 352696)
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 23.0
Assigned To: Magnus Melin
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-20 13:35 PDT by Magnus Melin
Modified: 2013-04-03 12:23 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed fix (10.03 KB, patch)
2013-03-20 13:35 PDT, Magnus Melin
no flags Details | Diff | Review
proposed fix, v2 (11.34 KB, patch)
2013-03-20 13:54 PDT, Magnus Melin
bwinton: review+
bwinton: ui‑review+
Details | Diff | Review
appmenu screenshot (20.88 KB, image/png)
2013-04-01 12:50 PDT, Magnus Melin
no flags Details

Description Magnus Melin 2013-03-20 13:35:00 PDT
Created attachment 727340 [details] [diff] [review]
proposed fix

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.)
Comment 1 Magnus Melin 2013-03-20 13:54:54 PDT
Created attachment 727354 [details] [diff] [review]
proposed fix, v2

This one.
Comment 2 Blake Winton (:bwinton) (:☕️) 2013-04-01 09:38:54 PDT
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.
Comment 3 Magnus Melin 2013-04-01 12:50:31 PDT
Created attachment 732000 [details]
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.
Comment 4 Blake Winton (:bwinton) (:☕️) 2013-04-01 12:57:24 PDT
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.
Comment 5 Magnus Melin 2013-04-03 12:22:57 PDT
http://hg.mozilla.org/comm-central/rev/68687817c22d -> FIXED

Note You need to log in before you can comment on or make changes to this bug.