Closed Bug 1609901 Opened 7 months ago Closed 6 months ago

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

Categories

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

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 74.0

People

(Reporter: bugzilla2007, Assigned: aleca)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

STR

  1. type |foo@bar.com, | (one trailing space - regular scenario if you intend to type |foo@bar.com, baz@bar.com|)
  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 (foo@bar.com)
  • 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

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.

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
Type: enhancement → defect
Assignee: nobody → alessandro
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → Thunderbird 74.0
Version: 73 → Trunk
Attached patch 1609901-polish-trimming.patch (obsolete) — Splinter Review

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

Attachment #9123743 - Flags: review?(mkmelin+mozilla)
Attachment #9123743 - Flags: feedback?(bugzilla2007)
Attachment #9123743 - Flags: review?(mkmelin+mozilla) → review+
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.
Attachment #9123743 - Flags: feedback?(bugzilla2007) → feedback+
Blocks: 1610883

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

Attachment #9123743 - Attachment is obsolete: true
Attachment #9123855 - Flags: review+
Attachment #9123855 - Flags: feedback+

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/10f51319aa7b
Remove unwanted whitespaces from compose message input fields. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Component: Composition → Message Compose Window
Product: MailNews Core → Thunderbird
You need to log in before you can comment on or make changes to this bug.