315144 - Pressing [Shift+]DEL on focused(!) attachment(s) within the mail window deletes message(s) instead of attachment(s)

Status: RESOLVED FIXED

(MailNews Core :: Attachments, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Still confirmable with my current cvs build from today.

David, this is another bogus focus bug. Although an attachment is focused the
whole message gets deleted when pressing Delete. At the time when you had
checked-in that new amazing feature the handling of Delete has changed. Now
Delete isn't only responsible for deleting a message itself. Now we have more
conditions, like the attachment stuff.

When pressing Delete we should first check where the focus is located. Is it
within the attachment box then only the selected attachment has to be deleted.
Otherwise we can safely delete the message.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Like David already mentioned in bug 256224 the fix won't happen for TB 1.5.

Comment 2

[Karsten Düsterloh]

This happens also in SaeMonkey, eg. 20051024.

[BTW: to avoid duplicate front end bugs, we really should consider having Core:Front End or some such...]

Comment 3

[Mike Cowperthwaite]

*** Bug 239683 has been marked as a duplicate of this bug. ***

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Quick proposal for Tb to circumvent message deleting

I've already talked to Neil on IRC and he stated to use controllers instead of event handlers. Also Tb and SM behaves different. While SM uses a listbox for the attachment list, Tb makes use of <description>. Due to that difference the patches will also differ.

David what is your suggestion for Tb? Should I also use controllers here? Could this patch be used to fix the issue for 1.5 or it is too risky?

Comment 5

[Scott MacGregor]

Henrik, 1.5 is all but wrapped up. We'd only respin if there was a big stop ship bug and this wouldn't qualify for that. Just an FYI in response to your 1.5 question. 

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #5)
> Henrik, 1.5 is all but wrapped up. We'd only respin if there was a big stop
> ship bug and this wouldn't qualify for that. Just an FYI in response to your
> 1.5 question. 

Scott, I just wanted to give it a try, not even more. But one question to you. When you had implemented that part why you have chosen the description element instead of using a listbox like in SeaMonkey? Does it include more features or what are the main reasons? Just for my appreciation, before I dive a bit deeper into that issue.

Comment 7

[Michelle]

My bug is very similar to this one so I'm not going to file a new bug.  I'm using 1.5 on OS X.  My difference is when this happens, the message just disappears completely, bypassing the trash.  I lost a mail message this way, and then several more unimportant ones just trying to reproduce it.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #7)
> using 1.5 on OS X.  My difference is when this happens, the message just
> disappears completely, bypassing the trash.  I lost a mail message this way,
> and then several more unimportant ones just trying to reproduce it.

What do you have selected within the server settings for this account? Will messages be deleted immediately when removing them? In that case it's a real dataloss bug which should be fixed ASAP. Michelle, to restore the message open the inbox file and search for the message if you don't have compressed that folder. Change the X-Mozilla-Status to 0x0001 to see the message again.

David, most of the people can't handle that way of restoring. We should fix this for the next release. 

Comment 9

[Michelle]

In my server settings I have set for the message to be moved to the trash upon delete.  Also I have tried to search for the message in the inbox when this happens, and it's not there, it's completely gone.

Comment 10

[Mike Cowperthwaite]

*** Bug 345212 has been marked as a duplicate of this bug. ***

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The new handling of attachments inside the mail window is nice. But this issue still exists. Hitting the delete key while an attachment is focused the entire message will be deleted.

Scott, David, is there no change to get this trivial thing into Thunderbird 2? Shouldn't we only have to make sure which element has the focus before calling the delete function?

This bug affects a lot of people and a patch for Tb2 will be highly honored.

Comment 12

[Thomas D. (:thomas8)]

(In reply to comment #11)
> The new handling of attachments inside the mail window is nice. But this issue
> still exists. Hitting the delete key while an attachment is focused the entire
> message will be deleted.
> 
> Scott, David, is there no change to get this trivial thing into Thunderbird 2?
> Shouldn't we only have to make sure which element has the focus before calling
> the delete function?
> 
> This bug affects a lot of people and a patch for Tb2 will be highly honored.
> 
My full and urgent support for the above, it will be very much appreciated if this CRITICAL(!) error could be fixed for TB2. It is the most natural thing in a world that when I press <del>, I expect only the object with the focus (the attachment) to be deleted and not some completely unrelated object from somewhere else in my UI (the message). Furthermore, this is highly likely to result in data loss, as people who are using the keybord regularly might just hit <shift+del> without second thoughts to make sure the attachment is actually expunged rather than copied to some trash location, and, due to the lack of (optional) confirmation dialog for <shift+del> as has often been complained about elsewhere, the whole message will be lost without warning even though I had my focus quite unambiguously and solely on the attachment pane!!!

Please assign this bug to someone and please try to include the fix in TB2!

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Wayne, dunno if you also familiar with the js stuff behind. But I think this issue will be simple to fix for you.

Comment 14

[Thomas D. (:thomas8)]

(In reply to comment #0)
> 
> When pressing Delete we should first check where the focus is located. Is it
> within the attachment box then only the selected attachment has to be deleted.
> Otherwise we can safely delete the message.

I just had a glance at Henriks proposed patch and assuming that "case KeyEvent.DOM_VK_DELETE" catches only the <del> key as such I suggest that <shift+del> should also be caught if attachment has focus. Keyboard addicts like me use shift+del automatically on anything which looks like a file object (and attachments DO look like a file...) to make sure things get expunged and dont stick in some trash location. It's one extra line of code in the patch to prevent data loss in this case, for with the current behaviour, pressing <shift+del> will still result in the message rather than the focussed attachment to get deleted.

Another question in this context: (catch <enter> pressed on focussed attachment)
I have been wondering since I first started using TB, why the ... is it not possible to use <enter> for the default action on a focused attachment? (dialog to open/save, or userdefined default action open/save). The above patch IMO would be the right place to catch that <enter> key when it's pressed while attachment has focus, by simply adding another "case Keyevent.dom---ENTER---" to the list (I don't know the right syntax, but you know what I mean).

Greets & thanks for any advances on that matter!

T.

Comment 15

[Thomas D. (:thomas8)]

(In reply to comment #14)
> I suggest that
> <shift+del> should also be caught if attachment has focus.
> Another question in this context: (catch <enter> pressed on focussed
> attachment)

Another reason why this bug has been marked critical is the following related bug (Ctrl+A when focus in attachment pane will unexpectedly select all messages rather than all attachments): https://bugzilla.mozilla.org/show_bug.cgi?id=164351
Danger of data loss becomes a new meaning in that situation in the following quite normal keyboard scenario:
- focus on first attachment (btw not possible with keyboard: <tab>bing skips the attachment pane???) 
- ctrl+a (expected behaviour: all attachments should be selected; actual behaviour in tb 1.5: all messages of current folder will be selected)
- [<shift>]+<del> (expected behaviour: all attachments should be deleted; actual behaviour: all messages will be deleted [permanently, in case of shift+del]

So <ctrl+a> would be another "case
KeyEvent.DOM..." to be added to Henrik's proposed patch attachment in #0.

btw Even if you possibly think that <shift+del> should not delete focused attachments (why not?), then at least please make sure that while the focus is on any attachment(s), <shift+del> should do nothing rather than deleting all msgs!

thx

T. 

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Sorry I actually don't have time to do any investigation for the proposed patch which is also quiet a long time ago. Anyone who could take it?

Comment 17

[Wayne Woods]

I'm looking at it. I've looked at the js behind delete before, and can probably do it (though whether I do it the *right* way is another matter ;) )

There's one conflict at the moment, however. That's because a selected attachment remains selected even after the focus has shifted back to the parent message in the threadpane, unless the user deliberately clears it by clicking on the attachmentpane outside the message, or selecting a new message. So I could select an attachment, then put the focus back on the message in the threadpane, and the attachment remains selected, and would be deleted in preference to the message. I'll see what I can do to fix this, anyway.

Comment 18

[Karsten Düsterloh]

I'd say it'd probably be best to funnel the delete action only to the attachment pane if that indeed has the focus. 

Comment 19

[Wayne Woods]

Attachment: Patch v2

Is this the direction the patch should be taking? This switches not only del, but also shift-del and the menu delete, but only when a attachment(s) are selected and also focused.

Comment 20

[Thomas D. (:thomas8)]

(In reply to comment #19)
> Created an attachment (id=258665) [details]
> Patch v2
> 
> Is this the direction the patch should be taking? This switches not only del,
> but also shift-del and the menu delete, but only when a attachment(s) are
> selected and also focused.

@Wayne: Did you include ctrl+a in your switches? I assume this would be the right and easy place to fix https://bugzilla.mozilla.org/show_bug.cgi?id=164351 and make sure that whenever
- FOCUS is in attachment pane (no matter if any or how many attachments are selected)
- and user presses CTRL+A
==> expected result: all attachments should be selected (and not all messages, as is actual result in TB 1.5!) to avoid risk of data loss?
(cf. my comment https://bugzilla.mozilla.org/show_bug.cgi?id=315144#c15)

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Scott, can you have a look at the patch from Wayne? It has been waiting for review for 2 month now. Thanks.

Comment 22

[Thomas D. (:thomas8)]

(In reply to comment #21)
> Scott, can you have a look at the patch from Wayne? It has been waiting for
> review for 2 month now. Thanks.

... 3 months ... oh, and it's critical - keyword: "dataloss"...

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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

Phil can you take the review process? Thanks.

Comment 24

[Phil Ringnalda (:philor)]

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

Yeah, I can review, but not until you have workable UI, and I can't quite picture what it's going to be.

I understand your desire to be able to delete attachments, but for most people, they get an email with an attachment, they read the email, double-click the attachment icon, look at the attachment, close the helper app, and delete the email. Requiring that they refocus the body, much less the current approach of requiring that they focus the empty part of the attachment pane and then focus the body of the email, before they can delete it, isn't going to fly.

Comment 25

[Wayne Woods]

(In reply to comment #24)
> Yeah, I can review, but not until you have workable UI, and I can't quite
> picture what it's going to be.

Thanks Phil. You may be right about it not being the best UI for deleting attachments from emails. But I don't think that's the major concern of this bug, which is more a matter of unexpected dataloss: that pressing delete when an attachment is selected deletes the entire message.

If you don't want Tb to delete attachments that way, I'm happy to alter the patch so that when del does nothing when an attachment is selected. That would be better than the current situation. What ya think?

Comment 26

[Thomas D. (:thomas8)]

(In reply to comment #24)
> (From update of attachment 258665 [details] [diff] [review])
> but for most people,
How do you know?? (Empirical data?) I'm sure most people would expect that when they press DEL on an explicitly selected AND focused(!) UI object, exactly and only that object (the attachment) and not some random containing object (the whole mail) gets deleted, because any other behaviour is a gross violation of even the most basic (Windows) OS GUI rules. How would you like it if Windows Explorer deleted your whole containing folder instead of the selected and focused file on which you hit DEL? Or how would you like it if Word dumped the whole document, instead of the highlighted word on which you just hit DEL, for the convenience and custom of some users? Oh yes, it might be somewhat more convenient for some people to delete whole folders when just a file has the focus, or whole documents when just a word is selected. But such a behaviour would go against any reasonable intuition that people have when they operate within Windows, or, for that point, any other OS. This is not just our "desire to be able to delete attachments": this is a BUG, and, furthermore, a DATALOSS issue (both for DEL and shift+DEL)! (I concede that toolbar buttons and menus might be a different question, see my compromise suggestion below).
 
> Requiring that they refocus the body...
...is a matter of a single click into msg body or SHIFT+TAB (to refocus msg preview pane)
 
> much less the current approach of
> requiring that they focus the empty part of the attachment pane and then focus
> the body of the email, before they can delete it
From my reading of the code (correct me if I'm wrong), this is NOT true: NO need for "deselecting" inside attachment pane, just one click (or SHIFT+TAB) straight away to refocus msg preview pane, then [SHIFT+]DEL to delete the msg. Delete action gets channelled to selected attachment(s) ONLY if the patch function  attachmentIsFocused() explicitly returns that "attachment pane (!) has focus (AND one or more selected attachment(s))"; so as soon as anything else gets the focus (regardless of attachment selection), msg deletion will work like it always did...

> isn't going to fly.
Dataloss Bugs fly much worse, IMHO...

Please find my compromise suggestion in the attachment below.

Comment 27

[Thomas D. (:thomas8)]

Attachment: compromise suggestion: small change for Patch v2 (toolbar delete button always deletes msg)

Here's a suggestion for COMPROMISE:
@Wayne: Looking at the tooltip of the toolbar BUTTON_DELETE ("Delete selected message or folder") and its location (far from attachment pane, next to message-related buttons like TAG and JUNK), what if you allow for the TOOLBAR BUTTON to ALWAYS delete the message (i.e. ignore attachment focus)? This way, Phil can have a single-click delete (without refocusing) for the open-attachemnts-and-delete-msg scenario, while the dataloss bug gets fixed in that pressing [SHIFT+]DEL on focused attachment(s) will delete these attachments only.
 
Assuming that /Case "button_delete":/ handles the toolbar button only, I am thus suggesting a one-line change over Wayne's patch v2 (attachment 258665 [details] [diff] [review], dated 2007-03-15), in my mini-patch dated 2007-04-17 (see attachment).

IMO, the menu (Edit > Delete) should reflect the currently focussed object, so the menu's caption should change dynamically from "Delete message" to "Delete attachment" in case individual attachment(s) are (selected and) focused. But even without the dynamic caption: it won't hurt so much for someone intending to delete the whole msg to find he's only deleted the focused attachment, but it DOES hurt if you intend to delete only the attachment and then find you've deleted the whole msg.

Comment 28

[Wayne Woods]

(In reply to comment #27)
> Here's a suggestion for COMPROMISE:

That sounds reasonable to me, but I want to hear what Phil thinks about the whole approach before spending any more time on the patch (v2 wasn't meant to be a final patch... I asked for feedback before proceeding further, but didn't get much. I probably should have flagged it for review earlier to get it attention).

At the very minimum, I think we're all agreed the delete mail operation needs to be suspended in the particular scenario covered by this bug. Whether del should delete the attachment in that situation is presumably up to Phil, though I'm in favour of it. If it's allowed, we need to be sure it's undoable as well.

Comment 29

[Thomas D. (:thomas8)]

(In reply to comment #28)
> ... Whether del
> should delete the attachment in that situation is presumably up to Phil, though
> I'm in favour of it. If it's allowed, we need to be sure it's undoable as well.

When deleting attachments with TB2 via attachment context menu, there's a confirmation msg (as it should!), warning that "This action cannot be undone". So attachment deletion in TB2 is generally not undoable yet. So the implementation of undo is a different issue which probably doesn't have to be a precondition for allowing intuitive attachment deletion with [SHIFT+]DEL.

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This is a critical data loss bug. Shouldn't it get more attention?

Comment 31

[David Ascher (:davida)]

phil, any thoughts on comment #27?

Comment 32

[Phil Ringnalda (:philor)]

Comment on attachment 270876 [details] [diff] [review]
compromise suggestion: small change for Patch v2 (toolbar delete button always deletes msg)

My primary thought is "yay, we've got a UX lead who has to make the call!"

Comment 33

[Bryan Clark (DevTools PM) [:clarkbw]]

yay, a human shield!

So I'm concerned about the heavy reliance on focus for this to work.  Though we don't have a concerted push towards this I'm trying to control what the Thunderbird single keyboard shortcuts do for messages.  Using the focus within the message to overwrite the actions of those shortcuts is ripe for appearing inconsistent in a way that this bug already has plenty of comments about.

In bug 231654 you can see a very similar issue of focus trying to overwrite the delete shortcut when a message is in view.  The basic rule of thumb here is that when a message is being displayed all single keyboard shortcuts go to that message (and can't be overwritten based on focus).

However I've had an idea for a possible direction that could be taken to handle these kinds of extra actions via the keyboard while keeping everything quick, keyboard accessible, and inline.  I'm not even an emacs users, but it consists of basically adding modes to keyboard shortcuts, visual modes.  I'll attach a screenshot of the basic concept in a minute so you can all laugh at me :)

Comment 34

[Bryan Clark (DevTools PM) [:clarkbw]]

Attachment: other keyboard shortcuts mode menu

So here's the concept for the "other actions" keyboard shortcuts mode menu.  Currently we have a pop up menu for "Other Actions", which is a flattened button that brings up a popup menu.  However that menu isn't going to make all the options keyboard accessible.

So what I've been tossing around is the idea that we could have an inline popup menu, as seen in the screenshot, for offering these "other actions".  The pop up menu is large and overlays the existing message; this gives a hint that we're no longer just working with the message.  The menu could be called up by using the "o" key for _o_ther and a single "c" key for _c_ancel.  Once the menu comes up we can offer a number of other keyboard commands for acting on a message in a way that isn't the standard items that need quick keyboard keys for everyone.  Each of the items in the menu have keyboard shortcuts as well as are available from mouse clicks.

So an example of the keyboard interaction for deleting all attachments could be: "o" (brings up menu), "d".  The menu closes after hitting "d" and the attachments are deleted through the normal interactions possible.

Anyway, the point here is to provide more keyboard accessible commands, but to rely less on focus and more on inline display of options while being very visible about a "mode change".

Comment 35

[Mike Cowperthwaite]

I don't see how this new menu addresses the basic issue here:  people see the focus on an attachment item, and are surprised when <del> deletes the message rather than the attachment.  All the affordances in the world are not going to make this problem go away.  It would be a better use of resources to make the attachment panel keyboard-accessible (bug 373996).

There is a basic issue here in that the visual display of focus is so tightly interleaved with the visual display of selection.  The attachment panel provides a means to select one or more; drag the selection (can't drag more than one item now); open, detach, or delete the items in the selection (that does work now, via the context menu).

So you're looking at either breaking the focus-means-getting-keystrokes interface, or removing focus from the attachment panel and then eliminating selection or inventing a new, non-focusing selection mode.

But you're right that the focus-centric solution to this bug is problematic.  Maybe the attachment box should be a non-selecting panel which allows single-item drags and opens, and with an extra button that opens a dialog with the attachments in a tree control (allowing attachments of attached messages to be seen hierarchically) with full keyboard access, multiple selection, and multiple item-drag.

But I don't think a panel like that melds well with the View Message Source and other actions.   I think the attachment problem is bigger than a menu of actions.

Comment 36

[Thomas D. (:thomas8)]

Thank you, Mike, for adding a voice of reason in your comment #35. I am surprised, to say the least, at the path this discussion is taking: Suggested "solutions" are getting ever more sophisticated and complex, but they seem to more or less bypass the very simple original problem that this bug is about. It is still about breaking one of the most basic and intuitive assumptions people can reliably make about operating their software (to quote Mike): "focus means getting keystrokes". Note that we are talking about focus, not selection. Select several attachments, do a single click back anywhere into the main message pain, and those attachments will still be selected, but the focus is back on the message pane. Press DEL there and the message goes to the bin, fine (as the message had focus, but attachments didn't). However, if main focus indicator (AND selection(s)) is on attachment(s), which are a PART of the message with their own set of exclusive actions, no one has yet explained to me on what grounds you intend to break the "focus-means-getting-keystrokes interface" and accept dataloss for the most basic usage patterns, just for the alleged "convenience" of some users. Having said which, it is not that I am generally against breaking principles for the sake of convenience as long as it is safe for intuitive use (see below).

Referring to Bryan's comment #33:

> So I'm concerned about the heavy reliance on focus for this to work.

Sounds like there is something wrong or bad about relying on focus? Is there really???

> Though we don't have a concerted push towards this I'm trying to control
> what the Thunderbird single keyboard shortcuts do for messages. Using the 
> focus within the message to overwrite the actions of those shortcuts is ripe 
> for appearing inconsistent in a way that this bug already has plenty of 
> comments about.

There are at least as many comments in this bug that state exactly the opposite: Disregarding an explicit focus and targeted keyboard interaction on individual parts of the msg (here: attachments) and overwriting it with a whole-msg-related shortcut is ripe for appearing inconsistent in a way that simply continues to accept accidental dataloss by violating basic user interaction patterns as they are accepted and therefore expected throughout the world of software! (comment #26 has more examples showing the absurditiy of violating the principle of "focus-means-getting-keystrokes" in the case of deleting).

> In bug 231654 you can see a very similar issue of focus trying to overwrite 
> the delete shortcut when a message is in view. The basic rule of thumb here 
> is that when a message is being displayed all single keyboard shortcuts go to 
> that message (and can't be overwritten based on focus).

If I understand Bryan correctly, his argument in the above quote runs like this:
"In bug 231654, we decided to fix it by ignoring the actual focus on some object (the folder) by assuming that the user intends to delete another current object (the message). So let's do the same for this bug: let's ignore the actual focus on some object (the attachment) and assume the user inteds to delete another current object (the message)."
Such logic is fundamentally flawed, as it ignores the fundamental difference of scenarios of the two bugs:

In Bug 231654, the actual focus is on a LARGER amount of data (the folder). User presses DEL. The fix now implemented ignores the folder focus, and deletes a SMALLER amount of data (the message) instead, as it it can be assumed with a good portion of probability that the user doesn't mean to delete the whole folder as his eyes are on the current message displayed. In other words, the underlying rationale of redirecting the focus is to avoid accidental dataloss. No harm is done, but inconvenience for some users is accepted in a trade-off against safety.

As opposed to this, in this bug (bug 315144), the actual focus sits on a SMALLER amount of data (the attachment). In spite of this, when user presses DEL, a LARGER amount of data (the whole message) gets deleted. Some users consider this current behaviour convenient, while others (with the backing of fundamental usage patterns throughout the software world) are shocked that the actual focus an SMALLER object is ignored and unexpectedly redirected to a LARGER object (the whole message). So the proposed fix here is to RESPECT user's actual focus on the SMALLER object (the attachment). Applying the SAME rationale as in the fix for bug 231654, this means: DO NOT redirect the focus of the delete action from the SMALLER object (the attachment) to the LARGER object (the msg) so as to avoid unintended dataloss! Go safe and delete only the selected attachments. *No harm is done, but inconvenience for some users has to be accepted in a trade-off against safety*. Safety that lots of other users have a right to expect because anything else... would violate the most basic principles of user-software interaction. It could be that simple.

So the alleged "rule of thumb" that all keyboard shortcuts should ignore the actual focus and be redirected to the message is an over-simplification/over-generalization that is not supported by the rationale behind the implemented fix for bug 231654.

I do think that Bryans "other actions" keyboard shortcut menu might be a brilliant idea to improve GENERAL keyboard accessibility; I agree with Mike's comment #35 though that it doesn't help for this bug. Unless you think that DEL on focussed attachment should pop up an overlay menu for the user do decide between attachment deletion and message deletion:

---------------------
What do you want to delete?
---------------------
|Selected _Attachemnt(s)
|Current _Message
|_Cancel
---------------------

Which adds an extra step for both kinds of users. I doubt very much that would help. Or would it? (against it, consider following confirmation dialogue for attachment deletion and feature request for confirmation dialogue before shift-deleting).

I love the simple truth and straightforwardness of Mike's comment #35:
> people see the focus on an attachment item, and are surprised when <del>
> deletes the message rather than the attachment. All the affordances in the
> world are not going to make this problem go away.
Back to square one.
Maybe we just keep trying somewhere along age-old privileges of convenience, rules of thumb and reinventing the wheel. Or maybe someone could just fix it.

I might be wrong, but looking at comments like Wayne Wood's comment #28, Henrik Skupin's comment #30, and David Ascher's comment #31, I had the impression that compromise comment #26 might still be a viable approach (we could talk about menu and button details, e.g. dropdown delete button?, once you've made up your mind how to deal with the focus). Sorry for the length, looks I got carried away...

Comment 37

[Thomas D. (:thomas8)]

Typo fix: find the compromise suggestion in comment #27

Comment 38

[Thomas D. (:thomas8)]

Bug 164351 (Edit | Select | All / CTRL-A, with focus in the attachment panel, does not select all attachments), posted as early as 2002, points out the same problem as this bug: Users will be confused as their deliberate and explicit choice of focus (in attachment panel) is disregarded and unexpectedly redirected to the message list. When you fix the focus issue for (Shift+)DEL, please could you fix Ctrl+A along the way!

Since Bug 164351 is posted against the suite, do we have to post a new bug against Thunderbird to make sure this will get fixed?

Comment 39

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 270876 [details] [diff] [review]
compromise suggestion: small change for Patch v2 (toolbar delete button always deletes msg)

No, the reliance on focus here is too problematic.  The next bug I'll get is that someone clicked on the attachment area, then the toolbar delete and it didn't delete the correct thing.

With the attachment area we have four areas of selection active in the window; one of which is the focus.  That's just too much to reasonably keep track of.  I know, I know, some of you can easily do that all the time and I'm proud of your abilities; however many others can't and I'm supposed to stand up for them.

I think the right fix for this is going to be changing the attachments area to not be a separate focus element in the message pane.  Then give the attachments area it's own actions section for acting on multiple attachments.

Comment 40

[Magnus Melin [:mkmelin]]

In almost all cases the message delete can be undone, so "dataloss" is an edge case, and I don't think this needs to block tb3.

I'm not sure what's the way forward here. How about just make hitting del do nothing for when the focus is in the attachments pane?

Comment 41

[Bryan Clark (DevTools PM) [:clarkbw]]

> I'm not sure what's the way forward here. How about just make hitting del do
> nothing for when the focus is in the attachments pane?

That seems like a reasonable step.

Comment 42

[Thomas D. (:thomas8)]

Bryan wrote, comment #39
> With the attachment area we have four areas of selection active in the
> window; one of which is the focus.  That's just too much to reasonably
> keep track of. I know, I know, some of you can easily do that all the
> time and I'm proud of your abilities; however many others can't and I'm
> supposed to stand up for them.

Bryan, if you really think users are unable to interpret a big blue square background around a big-by-default attachment icon as a focused element, then I suggest that we should abandon the idea of focus altogether and you integrate a mind-reading module into TB to ensure that normal users can get their work done when they deliberately place focus on an object of their choice with the intention of acting on it, and you decide that this is something so extraordinary that TB should just ignore it... Once again, quoting Mike's comment #35: "All the affordances in the world are not going to make this problem go away." I mean, the WHOLE system of interacting with software as we know it is based on FOCUS GETS KEYBOARD INPUT. Where is the problem?!??

I am aware that you weren't happy with the compromise patch I suggested. Fact is, that compromise patch would be a huge improvement over the status quo, and you are getting rid of the first half of a dataloss bug! In comment #39, Bryan says (against the proposed *compromise* patch of comment #27 that proposed to let toolbar delete button delete messages even when attachments are selected):
> The next bug I'll get is that someone clicked on the attachment area,
> then the toolbar delete and it didn't delete the correct thing.
You must be kidding. That wrong behaviour you fear to get complaints about is exactly what this bug is all about, and you are using the symptoms of this very bug as an argument against fixing it properly! Indeed, you are right: The compromise isn't good enough, because users can STILL(!) complain that TB disrepects focus. Like it or not, the only good and safe way out of this is to just do the most normal thing on earth: RESPECT focus and allow acting on focussed elements:

ON CONDITION THAT one or more attachments are SELECTED AND FOCUSSED,
--> let DEL and Shift+DEL delete selected attachments
--> let the toolbar DELETE BUTTON delete selected attachments (which requires the tooltip of that button to change correspondingly)
--> let EDIT > DELETE delete selected attachments
Done. Finito. No ambiguities. Rule-conforming.

I know that the length of my Comment #36 does not invite to read it, but maybe it's worth the time: As I already pointed out, it won't hurt for someone who intends to delete the whole message to find that he has just deleted a smaller PART of that message, namely an attachment (on which he himself has placed the focus!), whereas it DOES hurt for someone rightly expecting to delete focused attachments ONLY to find the WHOLE MESSAGE is gone.
Who should complain if TB just RESPECTS FOCUS like any other application in the world??? I don't see that many people will, and even if they did, there wouldn't be any basis for such complaints.

Same applies for CTRL+A (Seamonkey/trunk bug 164351).

Comment 43

[Bryan Clark (DevTools PM) [:clarkbw]]

Thomas, I'm interested in making sure people aren't confused by focus issues in the attachment area, which I think are very common due to it providing an additional focus area beyond the other 3 (folder pane, message list, message reader) that already exist.  bug 436555 looks like it is going to do a lot to improve this situation by changing the attachment area and how it works.  With the improvements to the attachment area, removing the del key seems like a good step to improving the overall usability.

Comment 44

[Thomas D. (:thomas8)]

1) We now have a new situation for this bug:
- Delete button is now on header bar (thus clearly associated with deleting the msg)

2) We still have the main problem of this bug:
- when focus is on attachment and user presses (shift+)del, surely we don't want to delete the message (that's dataloss for good, in case of shift+del)
- it has been a proposed blocker all along since 2006

3) Here's a new UI BEHAVIOUR PROPOSAL, in view of new situation described in 1)
I think this could make all of us happy, Bryan, Magnus, and me ;)
Basically, this just eliminates the bug and leaves everything else untouched.

IF (and only if!) FOCUS is on any selected attachment(s)
(where focus means dotted border, not just selection):

a) we can't use (Shift+)DEL to delete the /message/ (dataloss, this bug)
b) so then let's use (Shift+)DEL to delete the /selected attachment(s)/ (as that's what user expects)

c) header pane "X delete" button still deletes the current message (as that's what user expects, see 1) above
d) Edit > "Delete Message" menu still deletes the current message (as that's what the caption says)
e) Edit > "Delete Message" menu however should not show the "Del" Shortcut in the label in this particular case (since DEL will only delete attachment(s) right now)

f) optionally(!) ADD the following menu BEFORE existing "Delete Message" menu (only for so long as focus is on attachment(s)):
Edit > "Delete selected attachment   Del" or "Delete selected attachments   Del"
(we might already be able to create those captions dynamically, I think, from looking at attachment 258665 [details] [diff] [review])

g) optionally(!) ADD the menu described in f) AFTER the following existing menu:
File > Attachments > Delete All...
                     Delete selected attachment(s)

h) even if we don't implement f) or g), user can still use the following existing menu to delete any attachment (yet not specifically the selected one(s)):
File > Attachments > attachmentX > Delete

For a basic fix, we only need b) and e) since everything else remains as it is now. Simple, and no confusion at all.
Bryan, what do you think?

Comment 45

[Thomas D. (:thomas8)]

Bryan, this bug and some person who messed about here with lotsa lengthy comments would still love to hear your UI opinion now that the conditions for this bug have completely changed with the arrival of the new header pane (see my comment 44).

The good news is, you can actually have it your way AND get rid of my nagging...

The last solution which you accepted for this bug was Magnus comment 40:
> How about just make hitting DEL do nothing when the focus is in the
> attachments pane?
...and Bryan said in comment 41: "That seems like a reasonable step."

So apparently you agree that we should no longer delete the whole msg when attachment(s) have focus. Good.
Take it a tiny step further, and arrive at the "everyone happy" solution of comment 44 (better):

If we agree that we can't delete the whole msg when focus is only on the attachment, then why spoil it for those other users who actually /want/ to delete the attachment? When selection AND focus are on the attachment, and user deliberately presses DEL, let's just delete the attachment (rather than do nothing).

With that minor change, we can avoid unpredictable behaviour and unwanted dataloss while leaving everything else as it is now. All the other details we've been worrying about in this bug before no longer apply because of the relocated Delete Button on header pane, which will always delete the msg regardless of attachment selection or focus (as it does now). Even the menu will remain untouched; proposed changes of comment 44 are optional.

Requesting UI-review for comment 44 b).

Comment 46

[Thomas D. (:thomas8)]

FYI: In attachment-focus related bug 573901, comment 15, Bryan recommends this for reading:

> For anyone working on this you might want to read over this: 
> https://developer.mozilla.org/en/XUL_Tutorial/Focus_and_Selection
> ...and this might still have relevant info:
> https://wiki.mozilla.org/XUL:Focus_Behaviour

This bug clearly violates the following rules, as laid out in the XUL_Tutorial/Focus_and_Selection:
1 clicking on elements moves focus to that element
2 focus receives keyboard input
3 there can only be one focus at a time

Applied to this bug:
1 single-left-clicking on attachment will select and focus it
2 focus (and selection) is on attachment -> attachment receives DEL key input, and should be deleted
3 so we can't keep the focus on the message -> we can't delete the message

Change needed to fix this bug:
- when selection AND focus are on attachment, and user presses [Shift+]DEL, let's correctly delete the attachment instead of the message
- no other changes needed (we might want to polish a bit, along the lines of comment 44)

Awaiting ui-review+ from Bryan for that single and simple change...

Comment 47

[Dan Mosedale (:dmosedale, :dmose)]

I'd love to see a fix here, but we wouldn't hold 3.1 for this if it were the last bug standing for the same reason that we didn't in Tb3: message delete is easily undo-able.

Comment 48

[David :Bienvenu]

(In reply to comment #47)
> I'd love to see a fix here, but we wouldn't hold 3.1 for this if it were the
> last bug standing for the same reason that we didn't in Tb3: message delete is
> easily undo-able.

Shift delete is not undoable.

Comment 49

[Philippp]

Probably there should be more attention to the focus at other events, too:
For instance (TB 3.0.1 on Windows), when the attachments area is focused and I press Ctrl-A for selecting all attachments (because I want to drag them somewhere), instead all *messages* are selected.

Comment 50

[Thomas D. (:thomas8)]

Attachment: Proposed UI, new and simple - DEL on focused attachment(s) should delete attachments, not message(s)

-> New + Simple UI proposal now in Bryan's review queue

Comment 51

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 432035 [details]
Proposed UI, new and simple - DEL on focused attachment(s) should delete attachments, not message(s)

This ignores what I would consider the more average case where a person drags an attachment to the Desktop and then tries to delete the message.  If we turn off keyboard delete then nothing happens, if we delete just the attachment I think that would be an unexpected action.

Perhaps if you create a "attachment area mode" by some key combo which allows actions to be in the attachment area.  Otherwise I think we're going to confuse most other users.

Comment 52

[Thomas D. (:thomas8)]

Attachment: UI requirements to fix dataloss scenario of bug 315144 (minimal solution as favoured by Bryan)

(In reply to comment #51)

Bryan, thanks for coming back to this.

> a) If we turn off keyboard delete [on focused attachments], then nothing happens,
> b) if we delete just the attachment, I think that would be an unexpected action.

I understand that Bryan accepts a) to fix the dataloss problem of this bug (as he indicated in comment 41), but he doesn't like b). Having a) as a minimal solution would at least fix the dataloss problem of this bug.

Bryan, just to make sure, could you confirm that the minimal UI change as laid out in this attachment has your approval?

Comment 53

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 444209 [details]
UI requirements to fix dataloss scenario of bug 315144 (minimal solution as favoured by Bryan)

This seems fine for now to stop the data loss issue.  I think with bug 436555 we'll be able to create a much better keyboard nav experience such that the focus problem is less of an issue.

Comment 54

[Jim Porter (:squib)]

Fixed by bug 630759.