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

RESOLVED FIXED in Thunderbird 11.0

Status

Thunderbird
Message Reader UI
--
minor
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: squib, Assigned: squib)

Tracking

({regression})

8 Branch
Thunderbird 11.0
regression
Bug Flags:
in-testsuite +

Thunderbird Tracking Flags

(thunderbird9 fixed, thunderbird10 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 574239 [details] [diff] [review]
Fix this and test it

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+
(Assignee)

Comment 2

6 years ago
Checked in with the un-delete button fixed (good catch!):

http://hg.mozilla.org/comm-central/rev/4bd62c62b092
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Keywords: regression
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 11.0
Version: unspecified → 8
(Assignee)

Comment 3

6 years ago
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)

Updated

6 years ago
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+
Checked into branches:

http://hg.mozilla.org/releases/comm-aurora/rev/a5b1789406ad
http://hg.mozilla.org/releases/comm-beta/rev/489cc53f5ae3
status-thunderbird10: --- → fixed
status-thunderbird9: --- → fixed
You need to log in before you can comment on or make changes to this bug.