Closed Bug 702201 Opened 13 years ago Closed 13 years ago

Clicking the "Delete" toolbar button with an attachment focused should delete the message, not the attachment

Categories

(Thunderbird :: Message Reader UI, defect)

8 Branch
defect
Not set
minor

Tracking

(thunderbird9 fixed, thunderbird10 fixed)

RESOLVED FIXED
Thunderbird 11.0
Tracking Status
thunderbird9 --- fixed
thunderbird10 --- fixed

People

(Reporter: squib, Assigned: squib)

References

Details

(Keywords: regression)

Attachments

(1 file)

It turns out that the "delete" toolbar button issues the same command as the "delete" key on the keyboard, so hitting the "delete" toolbar button tries to delete an attachment if it's selected/focused. We should issue a different command for the button to tell these apart. Here's a patch to do this. The patch also removes some dead code that tries to disable the delete button for news messages, on the assumption that "delete" = "cancel" in news.

Since this is technically a change in the UX, I'm also requesting UI review on it.
Attachment #574239 - Flags: ui-review?(bwinton)
Attachment #574239 - Flags: review?(bwinton)
Comment on attachment 574239 [details] [diff] [review]
Fix this and test it

Review of attachment 574239 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, that makes much more sense to me, since the rest of the buttons in the toolbar act on the message (or at least, not on the attachments).  ui-r=me.

The code looks good too, aside from the question below.  r=me with that fixed or explained.

::: mail/base/content/mailWindowOverlay.xul
@@ +1833,1 @@
>           <toolbarbutton id="button-mark-undelete"

Should the un-delete still use cmd_delete?
Wouldn't that lead to us marking something as deleted, then trying to unmark it, and ending up deleting the attachment?
Attachment #574239 - Flags: ui-review?(bwinton)
Attachment #574239 - Flags: ui-review+
Attachment #574239 - Flags: review?(bwinton)
Attachment #574239 - Flags: review+
Checked in with the un-delete button fixed (good catch!):

http://hg.mozilla.org/comm-central/rev/4bd62c62b092
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Keywords: regression
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 11.0
Version: unspecified → 8
Comment on attachment 574239 [details] [diff] [review]
Fix this and test it

Since this is a regression, we should probably get it into aurora at least, and maybe beta too.
Attachment #574239 - Flags: approval-comm-beta?
Attachment #574239 - Flags: approval-comm-aurora?
Assignee: nobody → squibblyflabbetydoo
Attachment #574239 - Flags: approval-comm-beta?
Attachment #574239 - Flags: approval-comm-beta+
Attachment #574239 - Flags: approval-comm-aurora?
Attachment #574239 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.