Last Comment Bug 702201 - Clicking the "Delete" toolbar button with an attachment focused should delete the message, not the attachment
: Clicking the "Delete" toolbar button with an attachment focused should delete...
: regression
Product: Thunderbird
Classification: Client Software
Component: Message Reader UI (show other bugs)
: 8 Branch
: All All
-- minor (vote)
: Thunderbird 11.0
Assigned To: Jim Porter (:squib)
Depends on:
Blocks: 630759
  Show dependency treegraph
Reported: 2011-11-13 23:42 PST by Jim Porter (:squib)
Modified: 2011-11-21 05:45 PST (History)
0 users
squibblyflabbetydoo: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

Fix this and test it (7.62 KB, patch)
2011-11-13 23:42 PST, Jim Porter (:squib)
bwinton: review+
bwinton: ui‑review+
standard8: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description User image Jim Porter (:squib) 2011-11-13 23:42:58 PST
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.
Comment 1 User image Blake Winton (:bwinton) (:☕️) 2011-11-17 11:57:41 PST
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?
Comment 2 User image Jim Porter (:squib) 2011-11-17 23:45:52 PST
Checked in with the un-delete button fixed (good catch!):
Comment 3 User image Jim Porter (:squib) 2011-11-17 23:46:38 PST
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.

Note You need to log in before you can comment on or make changes to this bug.