Closed Bug 328293 Opened 15 years ago Closed 14 years ago

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

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set
trivial

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: bugzilla, Assigned: alfred.peng)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
Attached patch Patch v1 for thunderbird (obsolete) — Splinter Review
This bug also exists on the Mozilla suite.
Assignee: mscott → alfred.peng
Status: NEW → ASSIGNED
Attachment #214003 - Flags: review?(mscott)
=> Update the bug info: Hardware/OS/Version
OS: Windows 2000 → All
Hardware: PC → All
Version: 1.5 → Trunk
Scott, could you help to evaluate this bug?
(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.
Could it be possible to have both ?
*clicking attachment: single yes, all no.
*clicking area: single no, all yes.
Attached patch Patch v2Splinter Review
The patch fulfills both situation Serge listed. And:
* clicking multiple attachments: single no, all yes.
Attachment #214003 - Attachment is obsolete: true
Attachment #251510 - Flags: review?(mscott)
Attachment #214003 - Flags: review?(mscott)
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 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.
Attachment #251510 - Flags: review?(mscott) → review-
Attached patch Patch v3Splinter Review
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?
Attachment #258090 - Flags: review?(mscott)
(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).
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 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.
Attachment #258090 - Flags: review?(mscott) → review+
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?
QA Contact: front-end
Alfred, did this patch end up bit rotting? I noticed it was still open.
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.
mscott, could I check this in without a sr?
go for it!
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
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
VERIFIED on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a6pre) Gecko/20070628 Thunderbird/3.0a1pre ID:2007062803
Status: RESOLVED → VERIFIED
Alfred, this may have introduced a regression, see Bug 390071.
No longer blocks: 390071
Depends on: 390071
Depends on: 466060
You need to log in before you can comment on or make changes to this bug.