328293 - Save all, detach all and delete all are supurfluous for one attachment

Status: VERIFIED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Adam Kennedy]

When a message has only one attachment and you right-click the single attachment, the Save all, Detach all and Delete all options in the context menu are supurfluous and should not be shown.

To reproduce:

1. Send yourself an email with a single attachment
2 [review]. View the email when it arrives
3. Right-click on the attachment file at the bottom of the message
4. The supurfluous menu entries will appear in the context menus.

Comment 1

[Alfred Peng]

Attachment: Patch v1 for thunderbird

This bug also exists on the Mozilla suite.

Comment 2

[Alfred Peng]

=> Update the bug info: Hardware/OS/Version

Comment 3

[Alfred Peng]

Scott, could you help to evaluate this bug?

Comment 4

[Andreas Wuest]

(In reply to comment #0)

> When a message has only one attachment and you right-click the single
> attachment, the Save all, Detach all and Delete all options in the context menu
> are supurfluous and should not be shown.

I object. When there is only one attachment (either really only one, or only one which is not deleted/detached), you could still right-click in the attachment area but not on the attachment itself. This produces disabled "Save As..." etc. menu items. Therefore it is convenient to still have the "Save All..." etc. items enabled, so if you badly aimed at the attachment, you can nevertheless perform the operation, without having to close the context menu and reaim at the attachment.

Comment 5

[Serge Gautherie (:sgautherie)]

Could it be possible to have both ?
*clicking attachment: single yes, all no.
*clicking area: single no, all yes.

Comment 6

[Alfred Peng]

Attachment: Patch v2

The patch fulfills both situation Serge listed. And:
* clicking multiple attachments: single no, all yes.

Comment 7

[Scott MacGregor]

Alfred, shouldn't we be disabling these items instead of hiding them? I don't think we normally hide menu items for items that should be disabled. 

Comment 8

[Scott MacGregor]

Comment on attachment 251510 [details] [diff] [review]
Patch v2

Actually, in this particular case, I like hiding the menus.

Could some of this logic be simplified a bit, i.e.

openMenu.setAttribute('hidden', !canOpen);
saveMenu.setAttribute('hidden', !canOpen);
menuSingleSeparator.setAttribute('hidden', !canOpen);

ditto for:

     detachMenu.setAttribute('hidden', canDetach && canOpen);
      deleteMenu.setAttribute('hidden', canDetach && canOpen);
      menuSingleSeparator.setAttribute('hidden', canDetach && canOpen);

The patch looks good if we shorten some of the logic statements I think.

Comment 9

[Alfred Peng]

Attachment: Patch v3

I think to hide the menu can make things clear a little bit.

When I delete one attachment from the email, there will be still some zombie like stuff. Before the patch, I can right-click it and have a menu to select. After the patch, there isn't a menu shows up with right click. Is this a problem?

Comment 10

[Serge Gautherie (:sgautherie)]

(In reply to comment #9)
> After the patch, there isn't a menu shows up with right click. Is this a
> problem?

Do you mean that "Deleted:file.ext" entries can't be handled anymore ?
Then, yes, I think we should keep a mean to delete them (individually too).

Comment 11

[Alfred Peng]

The current situation is that the right-click menu for the "Deleted:file.ext" file is "save all", "detach all" and "delete all". I don't know how we can handle this file anymore...

One question is why the deleted file still shows in the attachment list?

Comment 12

[Scott MacGregor]

Comment on attachment 258090 [details] [diff] [review]
Patch v3

I think it's ok that you don't get the context menu anymore because none of the actions have any meaning in the menu anyway on the deleted attachment.

If the attachment is dettached then I would expect to still see the context menu. 

Serge, are you ok with that? If none of the menu items do anything anyway...seems ok to not show the context menu.

Comment 13

[Alfred Peng]

For the detached attachment, the context menu is still available. If I choose a normal attachment and a deleted one, the context menu is the same as multiple attachments.

I found another issue for the detached attachment in the trunk code. For TB 2.0b2, I can still open the detached attachment. But for trunk code TB version 3 alpha 1, there is an error to open the attachment. The error message: 

The file .../temp3.txt&filename=temp3.txt cannot be found. Please check the location and try again.

temp3.txt is the filename for the attachment. Is this an known issue or a new one?

Comment 14

[Scott MacGregor]

Alfred, did this patch end up bit rotting? I noticed it was still open.

Comment 15

[Alfred Peng]

mscott, as in comment 13, the open operation causes another issue after I detach the file. That makes the bug stay open for a while.

The patch still works fine. I'll check in it.

Comment 16

[Alfred Peng]

mscott, could I check this in without a sr?

Comment 17

[Scott MacGregor]

go for it!

Comment 18

[Alfred Peng]

Checking in mail/base/content/msgHdrViewOverlay.js;
/cvsroot/mozilla/mail/base/content/msgHdrViewOverlay.js,v  <--  msgHdrViewOverlay.js
new revision: 1.91; previous revision: 1.90
done
Checking in mail/base/content/msgHdrViewOverlay.xul;
/cvsroot/mozilla/mail/base/content/msgHdrViewOverlay.xul,v  <--  msgHdrViewOverlay.xul
new revision: 1.24; previous revision: 1.23
done

=>FIXED

Comment 19

[Magnus Melin [:mkmelin]]

VERIFIED on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a6pre) Gecko/20070628 Thunderbird/3.0a1pre ID:2007062803

Comment 20

[Scott MacGregor]

Alfred, this may have introduced a regression, see Bug 390071.