88633 - Attachments menu persists even when INBOX selected.

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[[e-mail] (gone - use [e-mail] instead)]

Build ID: 2001-06-29-14

Observed using IMAP, haven't checked POP3 yet.

Summary: Attachments menu persists even when INBOX selected.

Steps to Reproduce:

1.  Click on any message that has attachments.  We know it gets the attachments
sub-menu under File.  (actually sometimes it doesn't, but that is a different bug).
2.  Go back to the folder pane and click on INBOX.

Expected Results:

There is no "Attachments" sub-menu underneath the File menu item.

Actual Results:

There is an "Attachments" sub-menu item underneath File.

Comment 1

[[e-mail] (gone - use [e-mail] instead)]

I should mention I saw this on the trunk, haven't checked the nsbranch yet.  

Comment 2

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Clearly, the folder pane's INBOX has focus here.

Comment 3

[[e-mail] (gone - use [e-mail] instead)]

Saw this on Win2K, OS--> All. Plat--> All.

Comment 4

[Morten Nilsen]

Still an issue in 2001121300 linux

Comment 5

[esther]

reassigning to olga, she checks menu items for correctness based on selection.

Comment 6

[Henrik Gemal]

selecting inbox still keeps the mail selected. So the attachment menu is for the
still selected mail.

Comment 7

[[e-mail] (gone - use [e-mail] instead)]

Henrik, did you look at the screenshot?

Comment 8

[Mike Cowperthwaite]

A few comments about this bug:

1) I don't believe there is any other menu item in the 3pane top-level menu that 
disappears when inapplicable, and I don't think the Attachments menu item should 
disappear either.  If a single message with attachments is selected in the 
thread pane (with or without the message pane open), then the Attachments item 
should be enabled with the appropriate submenu.  In any other case, the 
Attachments item should be disabled and have no submenu.

1a) The Attachments item in the standalone message window should *also* not hide 
the menu, but disable it, if the message it's displaying does not have an 
attachment.

2) The nature of the persistence is: if you click on any item in the folder tree 
-- an account, a folder -- the menu item continues to be initialized with the 
last selected message's attachments.  The menu apparently changes only when a 
new message is *selected* -- in fact, the submenu persists in this situation:
 - Select non-attachment message; open File menu, Attachments item is hidden
 - Select attachment message; do NOT open File menu
 - Select some folder or account other than the current one
Open the File menu now, the Attachments item is *still* there!

I think I will take a look at fixing this bug, to make it behave per item (1) 
above.  Not for a couple weeks, tho.

Reducing severity, this is hardly blocking any function.  If you choose an 
action on an attachment where there shouldn't be any listed, the action performs 
fine.

Comment 9

[Mike Cowperthwaite]

Attachment: Patch: diff -u against 1.7b-0310

(Following up on my comment #8)
> 1) I don't believe there is any other menu item in the 3pane top-level menu
> that disappears when inapplicable, 

Well, I was wrong about this.  File|Get Next NN Messages  is only visible with
a newsgroup selected, and the Message menu has several additional items if a
newsgroup message is selected.	I don't think that's right, tho.

> I don't think the Attachments menu item should disappear either.

I still think this is the case.  However, there is an instance where I feel it
is advisable to hide the Attachments menu:  in the 3pane, when the Message Pane
is not visible (splitter collapsed).  This is because the Attachments Menu is
initialized from values that have been loaded into the Attachments Panel in the
displayed envelope.  If the envelope is not visible, then for every
initialization of the menu, you'd have to load the message and parse for
headers; this seems like a lot of work to do before a menu popup.

An alternative would be to disable the menu item if the envelope is not
visible, but this would leave open the obvious: "Why doesn't the attachments
menu work when I've got a message with attachments selected?"

I think in the typical use of hiding the message pane, it is hidden pretty much
all the time for that user; she will use a standalone message window to see the
message, and the Attachments menu will always be visible in that window.

> If a single message with attachments is selected in the thread pane 
> (with or without the message pane open), then the Attachments item 
> should be enabled with the appropriate submenu.  In any other case, the 
> Attachments item should be disabled and have no submenu.
> 
> The Attachments item in the standalone message window should *also* not
> hide the menu, but disable it, if the message it's displaying does not have
> an attachment.

This patch implements these changes, including hiding the menu only in the
3pane if the message pane is not visible.

Comment 10

[Scott MacGregor]

Comment on attachment 143608 [details] [diff] [review]
Patch:  diff -u against 1.7b-0310

adding a reviewer

Comment 11

[[e-mail]]

Attachment: Alternative

Although Mike has some good ideas I think his patch is over-complicated, so I
devised my own implementation; while the code to hide/show the attachments menu
is loosely based on Mike's patch the disabling code is completely different.

Oh, and I also renamed IsThreadAndMessagePaneSplitterCollapsed - Thunderbird
already uses IsMessagePaneCollapsed anyway.

Comment 12

[[e-mail]]

Comment on attachment 144740 [details] [diff] [review]
Alternative

Let's see if I've made any blunders ;-)

Comment 13

[Stefan Borggraefe]

Comment on attachment 144740 [details] [diff] [review]
Alternative

This doesn't disable the Attachment menu when you first select a messsage with
an attachment and then one with none. :-(

Comment 14

[Mike Cowperthwaite]

I agree, that is a cleaner way to handle the disabling of the menu item.  The 
advantages of experience...

Neil, does your patch work in the standalone window?  I think the reason I had 
added separate InitFileAttachmentsMenu() functions in messageWindow.js and 
msgMail3PaneWindow.js was because IsThreadAndMessagePaneSplitterCollapsed() 
(now IsMessagePaneCollapsed()) wasn't defined within the standalone context; 
therefore, I couldn't call it directly from mailWindowOverlay.js.
Altho I see it's being called in commandGlue.js, so I'm not sure any longer.

Comment 15

[[e-mail]]

Attachment: Addressed review comments

OK, so it looks like my way is a runner, once I sort out the issues...

Comment 16

[Stefan Borggraefe]

Comment on attachment 144828 [details] [diff] [review]
Addressed review comments

>-          message_menuitem.setAttribute('checked',!IsThreadAndMessagePaneSplitterCollapsed());
>+          message_menuitem.setAttribute('checked',!IsMessagePaneCollapsed());

Nit: please add a space between the two parameters. I know this wasn't you, but
if you change that line anyway... ;-)

Other than that, this looks good to me. :-)

Comment 17

[Mike Cowperthwaite]

Neil, your patch is completely reviewed; how about a checkin?

Comment 18

[[e-mail]]

I use tabbed browser as a sort of task list which is all very well until Mozilla
crashes on me (no I haven't installed recall.mozdev.org, perhaps I should) so
I'm guessing that something along those lines caused me to overlook this :-/ and
now the tree's frozen for 1.8a I'll probably forget again when it opens :-(

Comment 19

[[e-mail]]

Fix checked in.

Comment 20

[Mike Cowperthwaite]

Verifying this has been fixed; I see it working fine in
  Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/20040522

Thanks, Neil!

Comment 21

[Scott MacGregor]

Attachment: patch to fix this for thunderbird too