Bug 1613284 highlighted an issue in which the attachment pane was lacking proper accessible keyboard shortcuts to trigger the file chooser if a file was already attached.

A newly added button was introduced to fix the issue, but now the section doesn't look great.
Let's rethink that area and make it gorgeous and functional.

All you need to do is restore the functionality in TB 68 before the backported bug 1547699 broke it. It was totally functional.

The new button implemented here
https://hg.mozilla.org/comm-central/rev/e83059b0ea0e#l2.109
makes no sense whatsoever:

• The is already a button above the attachment pane. Why do you need two now? If there are no attachments yet, you can summon the pane with Alt+M. Then you can bring up the context menu with the Windows context menu key (if you have one), or Shift+F10, cursor keys down to "Attach File" - Voilà.
• There was already a shortcut to add attachments: Ctrl+Shift+A (not working on Linux, bug 269228/bug 1519348, maybe time to fix that).
• The button makes no sense since it's inside the attachment pane, so how will you get there?
• The shortcut key F now obscures the shortcut to the File menu

Sorry, but what happened in bug 1613284 is really very disappointing :-(

I don't think the exiting button was very obvious how it would work: would bet new users easily get confused especially as it was above the area it acted on, which is quite unusual.

I don't know why you think it's hard to get to a button inside the pane. All you need to do is click it.

I don't think the exiting button was very obvious how it would work: would bet new users easily get confused especially as it was above the area it acted on, which is quite unusual.

That attachment button has always been there since - I don't know - TB 3.6? Before it was in the toolbar on the left, then it was moved to the right above the area where the attachment pane opens. Can you show evidence that this has ever caused an issue, are the bugs reported?

I don't know why you think it's hard to get to a button inside the pane. All you need to do is click it.

Well, start a new message, now how to add an attachment? Your button isn't there, so you can't click it. Besides, the pane is small enough as it is, on Windows you can fit five attachments without resizing. I really fail to understand why there needs to be a second button duplicating functionality of an existing button which has been established for years if not decades. Additionally you can already click on the pane to add an attachment or use Ctrl+Shift+A.

More breakage in bug 1634946.

(In reply to Jorg K (CEST = GMT+2) from comment #3)

That attachment button has always been there since - I don't know - TB 3.6? Before it was in the toolbar on the left, then it was moved to the right above the area where the attachment pane opens. Can you show evidence that this has ever caused an issue, are the bugs reported?

Maybe it reported, but I'm not aware of a report, but that doesn't mean it was all good. That you click around and sometimes it willl bring up a file prompt is clearly unexpected.

Well, start a new message, now how to add an attachment? Your button isn't there, so you can't click it.

There's the toolbar button which then logically brings up the attachment picker.

Additionally you can already click on the pane

Not discoverable, and not working if you had no empty area to click on. Since the toolbar Attach button showed the pane last time, I would frankly have guessed clicking it again would toggle it hidden.
This is a problem where different combinations of clicking would also serve dual use as bringing up the file picker or not.

I agree with Jörg that we've lost a lot of the carefully and cooperatively crafted keyboard UX-design around Alt+M ("attachment pane view state machinery").
That's not necessarily prejudicial against having another 'Attach'-button inside attachment pane, but as for the original design, I don't think there was anything missing or amiss, as we have never heard users complaining.

(In reply to Jorg K (CEST = GMT+2) from comment #1)

All you need to do is restore the functionality in TB 68 before the backported bug 1547699 broke it. It was totally functional.

+1

For the avoidance of doubt:
Current release (68.7.0) is also broken wrt Alt+M, so cannot be used to understand the original fine-tuned UX-design (explained below).

The new button implemented here
https://hg.mozilla.org/comm-central/rev/e83059b0ea0e#l2.109
makes no sense whatsoever:

• There is already a button above the attachment pane. Why do you need two now? If there are no attachments yet, you can summon the pane with Alt+M. Then you can bring up the context menu with the Windows context menu key (if you have one), or Shift+F10, cursor keys down to "Attach File" - Voilà.

+1. In fact, it was even easier: Alt+M to show and focus attachment pane, press Enter -> voila: Attach File!

• There was already a shortcut to add attachments: Ctrl+Shift+A (not working on Linux, bug 269228/bug 1519348, maybe time to fix that).
• The button makes no sense since it's inside the attachment pane, so how will you get there?
• The shortcut key F now obscures the shortcut to the File menu
  Sorry, but what happened in bug 1613284 is really very disappointing :-(

+1 :-(

List of functions which were easily keyboard accessible via Alt+M (attachment pane access key)

There's another big problem for keyboard users caused by the removal of the access key for the attachment pane:
Alt+M access key for attachment pane (when it was still working, not working in release) used to give easy and discoverable keyboard access to the full range of attachment functionality, so we have lost direct access to all of the following (via pane context / attachment context menus):

• add attachment(s), web page (also for empty pane)
• rename attachments
• remove attachments
• reorder attachments
• remind me later

Existing UX: Localized access keys as de-facto shortcut keys in composition

As for the UX design, compose window already has localized access keys for many important spots and functions of the UI, so the attachment pane access key (Alt+M for EN) was not extraordinary in any way:

Obvious benefit: instead of pressing TAB a thousand times, these access keys in the primary UI are de facto shortcut keys for fast and easy keyboard access to important functionality, mostly via focusing important spots in the UI. There is absolutely no logical difference from a user's pov between using Alt+S for "Edit Subject" (technically an access key) and Ctrl+K for "Insert/Edit Link" (technically a shortcut key). That's implementation-level, irrelevant for users. In fact, access keys are the better shortcuts because they advertise themselves in the primary UI, and can be localized.

Per original design, this works like a charm, highly efficient for keyboard users:

UX-design of "Attachment Pane View State Machinery" via Alt+M: Access key = Shortcut key!

• Alt+M to summon the bucket (when not focused)
  • show and focus when hidden or minimized
  • focus when shown but not focused: classic access key scenario (like Alt+S for subject) - access key removed by Bug 1613284.
• Alt+M to dismiss the bucket (when focused)
  • hide empty bucket
  • minimize full bucket

In German localization, it's Alt+G, and the original implementation ensured that the trick will always work, see below.
Easy to memorize because a single key combination does the trick, with all the accessibility benefits for the full range of attachment functionality as listed above.

Access key == Shortcut key!? Notes on technical implementation

Access keys in the same context (composition), like shortcut keys, are unique by definition, for any localization. EN has Alt+M, German has Alt+G, and there cannot be anything else in the primary UI context using the same access key or shortcut key.
So it's a myth that we cannot technically use an access key as a shortcut key. Our internal technical implementation is irrelevant as long as it works. In this particular case, to achieve the above design, we need the following (working implementation before regressions):

• Localized Access key, e.g. (Alt+)M for attachment pane, but without control attribute (i.e. access key is displayed, but technically, it doesn't trigger anything)
• Shortcut key Alt+M for cmd_toggleAttachmentPane (internally, we reconstruct the shortcut key: Alt + AccessKey, so that localizers cannot break this).
• cmd_toggleAttachmentPane (technically triggered by shortcut key) provides the access key functionality exactly when needed, i.e. when attachment pane is shown, but not focused. User sees access key in interface, and when used, it does the right and expected thing (focus bucket). Conveniently, same key combo also works to show (and focus) the bucket when hidden or minimized - so this access key does the trick always, even when you don't see it. Again conveniently, when bucket already has focus, you can press Alt+M again to dismiss it.

(In reply to Magnus Melin [:mkmelin] from comment #5)

There's the toolbar button which then logically brings up the attachment picker.

Yes, the toolbar button we had for decades, first left, than right. So I don't understand your line of thought. If you use that button to add the first attachment (since your second button isn't there yet), why do you need a second button that appears after the first attachment has been added?

(In reply to Magnus Melin [:mkmelin] from comment #2)

I don't think the exiting button was very obvious how it would work: would bet new users easily get confused especially as it was above the area it acted on, which is quite unusual.

I guess it's supposed to mean 'existing' button (vs. 'exiting'; just to make sure we actually talk about the same thing without more misunderstandings...).

I don't see any confusion for new users, because that's the only available button by default when you start adding attachments, and if you want to add more, you just use the same button again.
I'm not against inline adding per se, but the current button fails to convince, hence this bug.

I don't know why you think it's hard to get to a button inside the pane. All you need to do is click it.

It may not be hard, but is it really helpful or required? What about the loss of vertical space?
I think this needs careful consideration of the overall workflow.

• Obviously we cannot simply move the existing [Attach | v] button into the attachment pane (in the following: abbreviated as AP).
• Probably we also wouldn't want two big buttons doing the same (one on toolbar, one inside AP), that's duplicate. Smaller (+) button might work.
• We could convert the existing toolbar button into a mere AP disclosure button: might work, but it's costly: one extra click each time you add the first attachment.
• Currently (in 68.7.0), there's a single big [Attach | v] button which reliably adds attachments at any time, and it's right there on top of attachment pane.
• There was a feature to click on attachment pane whitespace (still advertised by tooltip) as an alternative way of adding another attachment - unfortunately, it was removed for dubious reasons. OK, you claim that's not discoverable enough - not sure, but does it have to be as long as the big button in the same corner is still around? We could also make that more discoverable by changing mouse pointer to a paper clip or (+), and making the tooltip less volatile.

Other possible considerations:

• Do we really want to open another big can of worms with more regressions and bind lots of developer time by redesigning the entire attachment area now?
• Maybe better to finish lots of loose ends and improve the recipient area?
• How much UI/UX change do we want for a single release? I suspect the average Thunderbird user is pretty conservative; Email in itself is arguably a conservative way of communication. Remember, we had users complaining that CC/BCC are now found on the right side...

(In reply to Magnus Melin [:mkmelin] from comment #5)

(In reply to Jorg K (CEST = GMT+2) from comment #3)

That attachment button has always been there since - I don't know - TB 3.6? Before it was in the toolbar on the left, then it was moved to the right above the area where the attachment pane opens. Can you show evidence that this has ever caused an issue, are the bugs reported?

Maybe it reported, but I'm not aware of a report, but that doesn't mean it was all good. That you click around and sometimes it willl bring up a file prompt is clearly unexpected.

Yes, unexpected because it was a bug. Generally, "clicking around" is not a typical modus operandi. You helpfully fixed part of bug 1613004 that clicking on whitespace with selected attachments would prematurely fire attach dialog instead of deselecting first. Unfortunately, users also lost ability to click on whitespace for adding attachments, and it's now inconsistent in Daily (works on empty bucket, fails on full bucket). After deselecting, there is no reason to click on whitespace again unless it's intentional (for adding more attachments, as advertised by dynamic tooltip, feature removed in Daily). Admittedly, tooltip is a bit volatile - we can change that, and even change mouse pointer. To further guard against selection misclicks, we can easily do this: whenever you hold Ctrl or Shift whilst accidentally clicking whitespace, don't trigger attach dialog (even without selected attachments). With that, selection misclicks are really all but impossible - missing an entire attachment item as a click target is pretty hard.

Well, start a new message, now how to add an attachment? Your button isn't there, so you can't click it.

There's the toolbar button which then logically brings up the attachment picker.

Yes. Always logically brings up attachment picker - so why duplicate this functionality with another button?

Additionally you can already click on the pane

Not discoverable, and not working if you had no empty area to click on. Since the toolbar Attach button showed the pane last time, I would frankly have guessed clicking it again would toggle it hidden.

Hmmm, not really... I guess that's a wrong guess.

• The toolbar button is labeled "Attach" and does not look nor behave like a disclosure button in any way.
• It has a tooltip "Add an attachment", and triggers the dialog first before showing the pane (only if you really attached a file).
• I am yet to see a split disclosure button with a dropdown - split buttons offer a default action and alternative actions.
  So mistaking that for a mere disclosure button isn't easy imho.

This is a problem where different combinations of clicking would also serve dual use as bringing up the file picker or not.

Well, the primary click target for attaching is big attach button on toolbar. No one is forcing you to click on whitespace for adding attachments if you personally don't like that. For others, it's just convenience which rocks once discovered, and not hard to discover from the tooltips (which however should be less volatile).

(In reply to Alessandro Castellani (:aleca) from comment #0)

Bug 1613284 highlighted an issue in which the attachment pane was lacking proper accessible keyboard shortcuts to trigger the file chooser if a file was already attached.

Unfortunately, that's an incorrect and misleading representation of bug 1613284 as filed by myself, and allegedly "fixed" by others.
For the avoidance of doubt, everyone (because original design is so seriously broken everywhere):

Alt+M was never designed to trigger the attach file dialog. Whenever you see that, it's a bug, and I have never requested such behaviour!

• Bug 1613284 was filed to restore summoning and dismissing the attachment bucket via keyboard (Alt+M for en-US), as described in section "UX-design of Attachment Pane View State Machinery" of comment 6. Just that, summon and dismiss attachment pane via keyboard. For a shown but unfocused attachment pane, summoning translates to focus (hence access key, now lost).
• Patch of Bug 1613284, instead of restoring the carefully crafted and correctly working UX design, has crippled the UX by removing Alt+M accesskey, spelling doom for keyboard users as they lose intuitive access not just to attaching files, but many more actions per comment 6, section "List of functions ... via Alt+M".
• Wrt to the new duplicate [Attach Files] button inside AP, as Jörg said, we cannot use Alt+F access key for attaching files because that's taken by file menu. And attaching files already has a shortcut, Strg+Shift+A, and before Alt+M was removed as an access key, many other simple keyboard ways.

A newly added button was introduced to fix the issue
Well, per above explanation I think there might have been some misunderstandings here.

but now the section doesn't look great.
Let's rethink that area and make it gorgeous and functional.

That's good, but is this the right time for another big task?

The button was added mainly for keyboard accessibility since, but I do think it helps make think less of a guesswork there. Yes, it could look a bit nicer - that's this bug.

I hope we can avoid discussing Alt-M behaviour at length: it was clearly not discoverable. Broken for a year on release and no complaints is telling in that regard! What can I say: code wise it's just wrong to dual/cross use components in a way they weren't intended. It will lead to trouble you can't really get out of in any clean way.

Let me shed some light here.

The Alt+M shortcut was totally disfunctional and was causing the attachment pane to close/open at every keypress, alongside triggering the File selector. Not great.

The "extra" attachment button inside that list is necessary because the toolbar button is not keyboard accessible, and once the attachment pane is opened, there's not way to move the focus there and trigger the File selector via keyboard.

We can't assume nor give for granted that all users can click or use keyboard shortcut to interact with a section.

I suggested to add that button as a temporary workaround as I will redesign the attachment pane section, because in the current state, and the paradigm which it always had, is not great nor discoverable.

I have a proposal with design mock-ups to simplify and fix all the problems. Unfortunately, as we all have experienced this, I got sidetracked by hundreds of other bugs, so apologies for the issues.

Alex, can you please install the version indicated in the user story (or TB 60 ESR) and try it out (I just did). Please refer to (the long) comment #6 for a description of how it worked before de-XBL broke it. You weren't around when Thomas, myself and Aceman implemented all that in 2018 which later broke and then got removed completely.

In brief: Alt+M which is still listed on the View menu did a few things: Summon the bucket, close or minimise the bucket and also focus the bucket and give keyboard access. Once focused, Windows context menu key or Shift+F10 gave the context menu, Enter allowed to add attachments, as well as Ctrl+Shift+A on platform where that isn't broken.

Also note the function described in bug 1634946 which is now also broken.

EDIT: Now my memory comes back. All the attachment stuff was first shipped in TB 60 ESR:
https://www-stage.thunderbird.net/en-US/thunderbird/60.0/releasenotes/
Many improvements to attachments handling during compose: Attachments can now be reordered using a dialog, keyboard shortcuts, or drag and drop. The "Attach" button moved to the right to be above the attachment pane. The access key of the attachment pane (e.g. Alt+M, may vary depending on localization, Ctrl+M on Mac) now also works to show or hide the pane. The attachment pane can also be shown initially when composing a new message. Right-click on the header to enable this option. Hiding a non-empty attachment pane will now show a placeholder paperclip to indicate the presence of attachments and avoid sending them accidentally.

(In reply to Jorg K (CEST = GMT+2) from comment #13)

Alex, can you please install the version indicated in the user story (or TB 60 ESR) and try it out (I just did).

Sure, I'll take a look at it later tomorrow.

In brief: Alt+M which is still listed on the View menu did a few things: Summon the bucket, close or minimise the bucket and also focus the bucket and give keyboard access. Once focused, Windows context menu key or Shift+F10 gave the context menu, Enter allowed to add attachments, as well as Ctrl+Shift+A on platform where that isn't broken.

Thanks for the quick overview, this was very helpful.
This confirms my concerns regarding how much the working implementation relies on the Alt+M shortcut to do multiple things.
Too many outcomes depending on the visibility status of the bucket, breaks expectations and it's prone to breakage.
I think the Alt+M shortcut should be restricted to exclusively trigger the File chooser to attach a file.

Bare with me for a few minutes as I upload some mock-ups with in-depth descriptions on how we can solve our problems, improve the usability of this area, and hopefully simplify the code in the process.

Relocate the Attachment Pane

Let's relocate the Attachment Pane at the bottom of the compose area and remove the Attach toolbar button. With these changes we will:

• Maintain visual consistency with how we list attachments when reading a message.
• Enabling us to include the Attach button in the main focus ring.
• Remove the issue of visible/collapsed panel option as we don't need to save space in the header area.
• The pane is always there, easily discoverable, it doesn't interfere with any major spacing as we have plenty of real estate in the compose body.

Collapsed always shows important Info.

This change will help the user to track how many attachments he has in the message, and the size of the message.
Even if the pane is "collapsed" (the user doesn't see which files he attached), he can always quickly glance at the most important info he needs to know, without the necessity of revealing/opening the panel because it's in the way.

We also can get rid of that pretty unflattering gigantic attachment icon we currently show when the attachment pane is collapsed with files in it.

Attachment list

The user will be able to open/close the attachment pane by focusing on the Twisty icon via a regular focus ring, or maybe with a dedicated shortcut we can provide, limiting the Alt+M to only trigger the File chooser dialog.

The opened pane will show individual files in a similar way we currently do when reading a message.
In this screenshot I applied a more refined UI to improve readability and visual separation between elements.
Reordering, editing, detaching, and so on, will work as expected since this is still a richlist element.

Bonus improvement

Something we can consider for a follow up bug, is improving the visual response when a user drags a file on top of the compose message.

We should return a larger area to allow attaching a file, and not limiting it to the attachment pane area.
An overlay could appear which can react based on the type of file the user is dragging (eg. if it's an image, the Append inline option can be hovered).

Looks pretty nice!

Yes, looks good and modern. And we should synchronize the read message attachment list together with this refresh. Thanks to the attachmentList.css used in both it should be easy to do.

I think the Alt+M shortcut should be restricted to exclusively trigger the File chooser to attach a file.

Well, it was never intended to do that, that behaviour was regressed as a bug. As you can see on the View menu, Alt+M was always meant to drive the visibility/focus/status of the attachment pane. Ctrl+Shift+A was there to open the file chooser, amongst other options I already detailed. Please refer to the documentation here: https://support.mozilla.org/en-US/kb/keyboard-shortcuts#w_writing-messages where these two shortcuts are listed.

Your new proposal removes the "vertical" attachment pane in the addressing header and places it below the message to make it similar to the display of attachments in the message (preview) pane. A few comments:

• We are here due to the lack of keyboard control, so I'm sure you're keeping that in mind.
• The existing solution is great on reordering attachments, there is Move to top/up/down/to bottom. I can't quite visualise how reordering would look in the new horizontal scheme.
• Currently users can attach file by dragging when anywhere onto the addressing header, it doesn't need to be the attachment pane (which isn't even there at first). So the drop area is pretty big. Dragging onto the message body has a different function, at least for images, they will be embedded into the message.
• Outlook (and eM Client, BTW) show attachments at the top, as do other mail clients. It's possible that their publishers did some research into what's best, some app's, like FairEmail even have an option. I think you're changing the UI quite heavily. Given the time constraints, we're already at TB 78 Daily going to beta in a month, it might be prudent to delay the rework until TB 79.

(In reply to Alessandro Castellani (:aleca) from comment #17)

Created attachment 9145661 [details]

While this is looking awesome at first sight, I'm a bit worried how practical it will actually be for enterprise use cases, and the significant space costs of this implementation over the current vertical implementation sharing its space with the header, and the sharing ratio dynamically controllable by user via dragging splitter and/or temporarily minimizing attachment pane. Contrary to the false impression created on this bug, there was nothing overly complicated or amiss about the current attachment pane, only it was severely regressed including release version. Of course, improvements are always possible. With the proposed horizontal pane, we are also losing many benefits of vertical attachment list as Jörg pointed out, wrt systematic overview of attached files and intuitive ordering. The prototype example shown has an empty header with only one recipient type, and just a few attachments with very short file names.

• Can you please provide a screenshot featuring many more recipients, and/or recipients of different types (like 5 To recipients, 10 CC recipients, 20 BCC recipients), and an open horizontal attachment pane with 10 files with long file names, and a comparative screenshot with identical screen size showing the same in the current arrangement?

Like Jörg I'd think it may not be prudent to roll out another massive UI/UX change within this very short time frame.

Thanks all for the feedback, much appreciated.
I'm gonna answer in order starting from Jorg's comment.

We are here due to the lack of keyboard control, so I'm sure you're keeping that in mind.

I was considering this bug more of a generic improvement of that area, as the "Improve the UI and UX..." title suggests. But I agree that we should focus on specific aspects, so we can maybe have progressive patches uploaded here to tackle dedicated issues.

The existing solution is great on reordering attachments, there is Move to top/up/down/to bottom. I can't quite visualise how reordering would look in the new horizontal scheme.

We can keep the reordering solution the same, as that remains a richlist CE, but I think that this solution is better since the received message has a horizontal attachment list, so the order specified by the user perfectly matches the order in which a recipients visualizes those attachments (assuming he's using TB, but hey, why wouldn't he? :D).
I think this is a minor adjustment that doesn't create any major issue.

Currently users can attach file by dragging when anywhere onto the addressing header, it doesn't need to be the attachment pane (which isn't even there at first). So the drop area is pretty big. Dragging onto the message body has a different function, at least for images, they will be embedded into the message.

Ah, I never noticed it, sorry. I always assumed the drop area was limited to the attachment pane due to the lack of visual aids when hovering with a file. The drop area proposal is a further addition, which is totally extra to this bug, so we can totally ignore that proposal for now.

Outlook (and eM Client, BTW) show attachments at the top, as do other mail clients. It's possible that their publishers did some research into what's best, some app's, like FairEmail even have an option.

Gmail, ProtonMail, and other clients have the attachments at the bottom. It might be that the positioning was determined by research, indeed, but I think we should keep it consistent with how we display the attachments when reading a message. Anyway, we can explore it.

I think you're changing the UI quite heavily. Given the time constraints, we're already at TB 78 Daily going to beta in a month, it might be prudent to delay the rework until TB 79.

Sure, that sounds good to me.
I know I'm too often super optimistic, but since we have the attachment component used in the message pane, we should be able to reuse it and style it pretty quickly to have it landing in beta and gather some feedback. But I totally understand your concerns.

Onto Thomas's message.

I'm a bit worried how practical it will actually be for enterprise use cases, and the significant space costs of this implementation over the current vertical implementation sharing its space with the header, and the sharing ratio dynamically controllable by user via dragging splitter and/or temporarily minimizing attachment pane.

I don't think we should worry too much as resizing and collapsing will always work and be in place. The bottom area used by the attachment pane is very small, we can even shrink it a bit more as I left a bit of extra blank space, and it actually offers a better "preview" of what the user will see if attachments are present.

Can you please provide a screenshot featuring many more recipients, and/or recipients of different types (like 5 To recipients, 10 CC recipients, 20 BCC recipients), and an open horizontal attachment pane with 10 files with long file names, and a comparative screenshot with identical screen size showing the same in the current arrangement?

Sure, I will do that, as I think this solution improves a lot the spacing of the header area specifically in these cases, when multiple attachments and addresses are used, reducing the noise of the area and creating a better separation of concerns.

I will also focus on the shortcut breakage first, and then on the UI improvements after, once we reach an agreement on the shortcut.

Cheers

(In reply to Alessandro Castellani (:aleca) from comment #23)

I think you're changing the UI quite heavily. Given the time constraints, we're already at TB 78 Daily going to beta in a month, it might be prudent to delay the rework until TB 79.

Sure, that sounds good to me.

Then we should look that the actual attachmentPane looks better. Initially there isn't much space for the attachments with this button in this area. Maybe a small button with only a "+" in it and area around the button get the same background colour as the attachment list and no border between to make the area visually bigger.

Quick-n-dirty mock-up.

Richard, the button was added for keyboard accessibility since the shortcut/access key was removed; the "+" button doesn't fulfil that requirement, or does hitting "+" trigger it? How about just restoring established TB 60 functionality here and leaving experiments for later?

Ah, okay. Then +1 for restoring previous functionality.

(In reply to Richard Marti (:Paenglab) from comment #24)

Created attachment 9146099 [details]
Then we should look that the actual attachmentPane looks better. Initially there isn't much space for the attachments with this button in this area. Maybe a small button with only a "+" in it and area around the button get the same background colour as the attachment list and no border between to make the area visually bigger.

Or we could just fix full keyboard access to all attachment pane functions via Alt+M, context menus as it was before and fix Ctrl+Shift+A which already exists as a cross-localization shortcut for attaching files. Mouse users will still have a big [Attach | v] button from toolbar right there on top of the pane.

If we insist on adding a duplicate button for mouse users, let it be on the right, please, and it should overlap any existing attachments with a bit of margin around the button, so that we have more space for what is really important in this pane, existing attachments.

If we insist on adding a duplicate button for mouse users, let it be on the right, please, ...

I suggest not to open this can of worms now. If you have "+", why not have "-" as well? Oh, and "up-arrow" and "down-arrow" to reorder, right ;-) - And then you truly double up even more of the functionality from the context menu. Personal note: I've never used any buttons: I drag and drop, drag and drop to change order, and select and delete to remove. So losing space where I could see an attachment is not a positive change. If we had telemetry ...

This patch fixes and polishes keyboard access to the full functionality of attachment pane (as discussed).

(In reply to Jorg K (CEST = GMT+2) from comment #1)

All you need to do is restore the functionality in TB 68 before the backported bug 1547699 broke it. It was totally functional.

(In reply to Thomas D. from comment #6)

+1

(In reply to Richard Marti (:Paenglab) from comment #27)

Ah, okay. Then +1 for restoring previous functionality.

(In reply to Alessandro Castellani (:aleca) via PM 2020-06-05)

Thanks ... for the patch.
I tested it and it seems to be working properly.

(In reply to Thomas D. from comment #30)

Created attachment 9146850 [details] [diff] [review]
This patch fixes and polishes keyboard access to the full functionality of attachment pane (as discussed).

Please note pre-existing access bugs:

• With full bucket shown, no keyboard access to the attachment pane whitespace context menu. Expected (inbuilt for richlistbox!): after deselecting all items, context menu key or Shift+F10 must open the whitespace context menu (cf. Win Explorer), not the item context menu. Users can still use Ctrl+Shift+A to attach files on Windows (see below). Pane context menu works perfectly for empty bucket (context menu key / Shift+F10 from focused, empty bucket).
• Someone said that Ctrl+Shift+A keyboard shortcut for "Attach File(s)..." is broken for MAC and Linux. Works perfectly on our biggest platform, Windows. (On a focused, empty bucket, you can also attach via pane context menu per above, or just press Enter to attach.)

We need to fix those on their own bugs (haven't checked for them), as well as bug 1634946.

(In reply to Thomas D. from comment #31)

• Someone said that Ctrl+Shift+A keyboard shortcut for "Attach File(s)..." is broken for MAC and Linux. Works perfectly on our biggest platform, Windows. (On a focused, empty bucket, you can also attach via pane context menu per above, or just press Enter to attach.)

It works on Linux and Windows. Mac not.

Working on Linux? What about bug 269228/bug 1519348? Surely it can be made to work on Mac as well then.

Furthermore, the CTRL+SHIFT+A shortcut doesn't work if the focus is on the body of the message, ...

It does in TB 68. If it's not working now, that's another bug. Did I suggest to try TB 60 ;-) ?

... nonetheless we should leave this bug open and use it to rebuild this whole area from the ground up.

In a new bug, please.

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is -- (Backlog,) indicating it has has not been previously triaged, the bug's Severity is being updated to -- (default, untriaged.)

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is -- (Backlog,) indicating it has has not been previously triaged, the bug's Severity is being updated to -- (default, untriaged.)

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is -- (Backlog,) indicating it has has not been previously triaged, the bug's Severity is being updated to -- (default, untriaged.)

The severity of these bugs was changed, mistakenly, from normal to S3.

Because these bugs have a priority of --, indicating that they have not been previously triaged, these bugs should be changed to Severity of --.

(In reply to Alessandro Castellani (:aleca) from comment #35)

Review of attachment 9146850 [details] [diff] [review]:

Thank you for the review! I fixed the nits as far as technically possible.

• If the pane is empty, hitting Enter will trigger the file chooser.

Which rocks, super efficient, easy to remember, and no harm, as there's no other reason to press Enter on an empty focused bucket (keyboard equivalent of click on empty bucket to attach).

• If the pane is not empty, hitting Enter will download the first attachment.

No choice here because of Focus 101: Focused element receives keyboard input. No surprise when pressing Enter on a focused attachment will act on that. Predictable for keyboard users, not inconsistent. Also note, certain users/workflows might never add more attachments to full bucket. But you can always use Ctrl+Shift+A for adding attachments, regardless whether bucket is empty or full. And if it wasn't for another bug, you could also always use attachment pane whitespace context menu to attach via keyboard (context menu key / Shift+F10 works on empty bucket; bug on full bucket with deselected attachments: shows attachment context instead; i.e. pane context is keyboard-inaccessible).

Furthermore, the CTRL+SHIFT+A shortcut doesn't work if the focus is on the
body of the message

It does work correctly on Win10 which I tested, with focus anywhere in the composition window, release 68.8.0 (32-Bit)-de and Daily 78.0a1 (2020-05-07) (64-bit)-en.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +813,4 @@

     return !gWindowLocked;
   },
   doCommand() {

•    toggleAttachmentPane("toggle", event);
  

The linter doesn't like this as it reports Unexpected use of 'event'.

Can we tell the linter to ignore this line because it works correctly?
I believe we cannot pass event through the call chain, due to Bug 461578 / Bug 959494.
However, along bug 461578 comment 5, the nsIDOMXULCommandEvent inherits the modifier keys of the original keyboard/mouse events which triggered the command event. So I tried several things and I couldn't find a better way of picking up the command event along the complex onCommand chain. Aceman's request Bug 959494 Comment 10 for more information about correct use was ignored.

@@ +6351,5 @@

•  menuitem.setAttribute("checked", "true");
  

• } else {

•  menuitem.removeAttribute("checked");
  

• }

I think we can simplify this with:
menuitem.checked = aAction == "show";

Unfortunately, menuitem.checked is not implemented as a JS property(1) - only implemented as attribute - , so it's undefined on inspection, hence my change to make it work. Otherwise setting the menu checkmark will fail for all non-menu calls of this routine, and we have plenty of those.

(1) https://developer.mozilla.org/en-US/docs/Archive/Mozilla/XUL/menuitem

Unfortunately, menuitem.checked is not implemented as a JS property(1) - only implemented as attribute - , so it's undefined on inspection, hence my change to make it work. Otherwise setting the menu checkmark will fail for all non-menu calls of this routine, and we have plenty of those.

What about using menuitem.toggleAttribute("checked", aAction == "show"); ?

(In reply to Alessandro Castellani (:aleca) from comment #43)

What about using menuitem.toggleAttribute("checked", aAction == "show"); ?

Had checked that already, doesn't work for me either.

Well, the Fluent stuff is the trick here since it allows this elegant solution: .key = { attachment-bucket-count.accesskey } - Besides, messengercompose.ftl is already populated with the pill stuff.

If that string is necessary and, based on the localization note, we don't want that to be translated, we should only add that since it doesn't affect previously translated strings.

Already having the messengercompose.ftl file doesn't affect the need of having a fluent migration recipe, as those strings are completely new and never previously translated before the 69 cycle.
A migration recipe is necessary when moving strings that were previously translated from a DTD or Properties file.

(In reply to Alessandro Castellani (:aleca) from comment #45)

Do we have a bug already opened for the Shift+Ctrl+A upload shortcut not
working on Linux when the body has the focus?

From https://mzl.la/2ZtiYFN, there's this:
Bug 1500882 - Ctrl+Shift+A to attach File(s) requires Caps-Lock on Linux - which I just duplicated to:
Bug 269228 - Ctrl+Shift+[0-9a-f] Shortcuts on text fields aren't accepted in GTK (shift+ctrl keys don't work) e.g. [Edit->Select->Thread] - which is claimed to be the Thunderbird duplicate of this 'mozilla bug':
Bug 186789 - Ctrl+Shift+[0-9a-f] Shortcuts in Mozilla conflict with ISO 14755 Input methods (shift+ctrl keys don't work) e.g. [mark all read]

Address nits of comment 45 (as explained on chat).

Nice. Moving over to bug 1634946?

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/d8ad92570434
Fix and polish attachment pane keyboard access. r=aleca

Sorry, this is about to be backed out because it broke two of the tests. Here's an example log: https://treeherder.mozilla.org/logviewer.html#?job_id=304077109&repo=comm-central

(In reply to Geoff Lankow (:darktrojan) from comment #53)

Sorry, this is about to be backed out because it broke two of the tests. Here's an example log: https://treeherder.mozilla.org/logviewer.html#?job_id=304077109&repo=comm-central

My bad. This patch deselects all attachments on blur (in line with Aleca's whitespace/less noise philosophy), so when the tests focus attachment pane, they no longer find a selected attachment, it'll be focus on one attachment only. It should still be focus on the same item, but maybe it's about selection. Unfortunately, I am busy with other things today.

I'll take care of fixing the tests.

Let's land the strings only first so that we don't miss string freeze.

We shouldn't do this.
The patch needs some test fixing, which I can do today, no need to split the patch to only land the strings.

(In reply to Jorg K (CEST = GMT+2) from comment #60)

Comment on attachment 9152633 [details] [diff] [review]
There's nothing wrong with landing new strings separately. They need to land
before the merges on Monday, 1st of June.

I agree. In fact, it's much safer to land strings-only, because as we have just seen, the full patch is always at risk of being backed out if there's something wrong, then we lose the strings also if they are combined.
It's also easy to split out from the main patch because all new strings are in messengercompose.ftl. The old strings remain, so this looks absolutely safe to me, assuming that our systems are not yet clever enough to choke on unused new strings.

So is there any reason against setting checkin-needed for the strings?

Tests fixed.

The browser_attachment issue was due to the fact that we were clearing the selection when closing the attachment pane, and the test expect the same selection maintained when reopening the pane, which is correct.
Clearing the selection doesn't have anything to do with visual noise or white space since it's not visible when the pane is collapsed, and the selection should be maintained to guarantee consistency in the interaction.

The browser_focus test was failing because after adding an attachment, we keep the focus there, which is correct, so on first TAB, the focus moves to the message body, which is correct and respects the current focus ring.
The test was expecting the focus to start on the To field, even after adding an attachment.
I fixed it by moving the focus on the To field after the attachment is created in order to properly follow the starting point of the tested focus ring.

Sadly the interdiff doesn't work any more, so here are Alex' changes.

I don't think Magnus wants to review the entire patch, so I'm happy to give my approval to those changes if Thomas agrees.

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/064f93c55fc4
Fix and polish attachment pane keyboard access. r=aleca,jorgk

Yup, sounds good.
The patch was already r+ and landed, so my diffs was only to fix the tests.
Thanks for landing it.