Pills' contextual actions (cut/copy/delete) are connected in odd ways, and passing a focus-pill element argument which isn't really needed and sometimes ends up nowhere.
Let's create a bit of order, simplify the call chain and convert them into generic internal functions of <mail-recipients-area>.
This will also make it easier to expose these actions on the main menu, e.g. for bug 1687432.

The patch.

• Simplify pills' contextual cut/copy/delete call chain by eliminating unneeded
  no-op focus pill element argument, which will make them context-independent.
• Make pills' cut/copy/delete actions internal functions of <mail-recipients-area>.
• Hook up pills' actions in the layout layer to generic onCommand handlers in
  MsgComposeCommands.js.
• Specify focused pill as default parameter for pill argument of
  removeSelectedPills().

(In reply to Alessandro Castellani [:aleca] from comment #2)

 removeSelectedPills(

•  pill = this.querySelector("mail-address-pill:focus"),
  

Mhh...I'm not sure about this, it feels weird.
We never use a selector as a fallback default attribute.
This is a purely code styling nit, I think we should ask Magnus about it.

This is terse and correct, because the focused pill selector is the default argument for the pill argument.
I understand that we are encouraged to write terse and correct code.
The alternative would be this:

if (!pill) {
  pill = this.querySelector("mail-address-pill:focus"),
}

That's 2 extra lines and less readability because you can't tell from looking at the function description or the arguments that pill has a default argument. I don't see why we should ask for doing the correct thing.

I don't see why we should ask for doing the correct thing.

I'm not saying that that's not correct, i was simply questioning the code formatting as, as far as I know, we never used that styling, and keeping code styling consistent across a large application is important.
Usually, we do this:
let selectedPill = pill || this.querySelector("mail-address-pill:focus").

As I said, it's a nit so if you wanna push it go ahead.

(In reply to Alessandro Castellani [:aleca] from comment #4)

I'm not saying that that's not correct, i was simply questioning the code formatting as, as far as I know, we never used that styling, and keeping code styling consistent across a large application is important.

Absolutely!

Usually, we do this:
let selectedPill = pill || this.querySelector("mail-address-pill:focus").
As I said, it's a nit so if you wanna push it go ahead.

Sorry, that was Friday evening, I must have been grumpy! That lengthy default parameter was looking a bit weird indeed.
Thank you for insisting on consistent code formatting!
On second thoughts, we can just rip that pill argument out, which will declutter a number of callers!

2nd iteration, more cleanup!

• Remove focused pill argument from removeSelectedPills(), as we can always retrieve the focused pill from inside this function. Typically as long as any pills are selected, the focus will also be on a pill (which may or may not be selected). Even when using context or main menus, the pill focus is not lost.
• If there's no selected pill, there's nothing to remove, so let's bail out.
• I'm now using the first selected pill as a fallback for the unlikely event that there's no focused pill.
• Declutter callers: removeSelectedPills(), moveSelectedPills().
• Shrinked dropAddressPill() to a one-liner with a single argument, the label where the drop occurs. This function was calling moveSelectedPills(mailRecipientsArea.getAllSelectedPills()[0],...) - not ideal, to iterate an unlimited number of pills just to get the first one... In fact, 5 out of 6 lines in that function were just clutter, as well as 2/3 of its arguments. I'm still not happy with the general design around dropAddressPill(), but at least it's now de-cluttered.

Complete revised summary of this patch:

• Simplify pills' contextual cut/copy/delete/move-to-* call chain by eliminating
  no-op focus pill argument, which will make them context-independent.
• Make pills' cut/copy/delete actions internal functions of <mail-recipients-area>.
• Hook up pills' actions in the layout layer to generic onCommand handlers in
  MsgComposeCommands.js.
• Retrieve the focused pill via querySelector at the only place where we need it,
  removeSelectedPills(), and handle lost focus gracefully.
• Cleanup dropAddressPills() and remove unneeded arguments; more to be done.

Thank you for the appreciative review!

Two linting nits fixed, otherwise unchanged, forward aleca's r+ from comment 7.

Remove a leftover focus pill function comment, and 2 other nits (rename item -> pill). r+ from comment 7.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/5ba627148586
Convert pills' contextual actions (cut/copy/delete) to generic internal functions of <mail-recipients-area> CE, and related cleanup. r=aleca