1609901 - Space in recipient's originalInput hard to remove with Backspace as it's falsy - polish trimming

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect, P1)

Description

[Thomas D. (:thomas8)]

STR

1. type |[e-mail], | (one trailing space - regular scenario if you intend to type |[e-mail], [e-mail]|)
2. press backspace to remove the excess leading space

Actual

• bug 1609894: premature pill creation
• space remains at the beginning of new input
• backspace focuses pill ([e-mail])
• hard to remove the space

Expected

• even after we fix bug 1609894: don't focus pill on backspace if there's spaces in input
• investigate our options for trimming/preventing initial spaces, but space is a valid character of local part if quoted, and for other scenarios, we do auto-quote I think

Comment 1

[Thomas D. (:thomas8)]

Oh, and if you press Enter after one or more spaces, it remains in the input, but shouldn't - spaces-only should definitely be trimmed away on Enter/blur etc.

Comment 2

[Thomas D. (:thomas8)]

Maybe we could prevent entering space as first character of a new input, because

• email addresses with leading space are highly unlikely, and only valid if quoted
• for most users, preventing leading space will be helpful
• for the exception, we can expect users to quote their special email address with leading space

Comment 3

[Alessandro Castellani [:aleca]]

Attachment: 1609901-polish-trimming.patch

This should fix this annoying bug.

• Don't intercept backspace, arrow left, delete, or CTRL+a, if whitespace is present in the input field, allowing user to select it and delete like a normal input field.
• Handle the Enter keypress on the input in order to focus on the next focusable element, and not jump on the Subject line if other addressing rows are visible.

Try-run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=851f148a3f703bb3c7a71c3cf2607955e1f3c6ef

Comment 4

[Thomas D. (:thomas8)]

Comment on attachment 9123743 [details] [diff] [review]
1609901-polish-trimming.patch

Review of attachment 9123743 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you! Haven't tested, but this looks good. Two nits.

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +503,1 @@
>      case "End":

I think this case should be removed. It feels weird that END on empty input with text cursor selects the last pill (moving leftwards!), and I don't think anyone would expect that (whilst a simple cursor left will do the trick). Instead, just do nothing. Note that all the other cases in this group are moving leftwards (Home, ArrowLeft, Backspace).

@@ +519,2 @@
>      case "Enter":
>        // No address entered, move focus to Subject field.

// No address entered, trim input and move focus to another recipient field
// if available, otherwise to subject field.

Comment 5

[Alessandro Castellani [:aleca]]

Attachment: 1609901-polish-trimming.patch

Patch updated and a new try run launched as a couple of bct2 were failing, but not locally.
Let's see what's going on here: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=0ba925a96b7d42840ba6c44dc1070cabb381db82

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/10f51319aa7b
Remove unwanted whitespaces from compose message input fields. r=mkmelin