1698484 - Comma pill creation takes full input instead of just the content before the typed comma

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Henry Wilkes (they/them) [:henry]]

When typing in the address input, a typed comma may trigger an autocomplete. The current behaviour works well when typing at the end of the input. But it is a little weird when typing at an earlier position. For example, if the user makes a mistake and misses the comma, and then goes back in the input to type it in.

Examples

In these examples we'll type a comma with different selections.

• | marks the selectionStart or selectionEnd of the input.
• || marks the cursor position, since selectionStart and selectionEnd are equal.
• [ and ] mark pill boundaries.

1. Autocomplete suggestion

This already works fine.

Set up:

s|uggested autocomplete <a@org>|

Current & Expect after typing comma:

[suggested autocomplete <a@org>]||

2. Cursor not at end

This is the weird behaviour.

Set up:

a@org||de

Current after typing comma:

[a@orgde]||

Expect after typing comma:

[a@org]||de

3. User selection

In situations where the user makes their own selection, it would be hard to tell whether they want an autocomplete or the normal "replace with key" behaviour.

When the user selects up until the end of the input, then this will look like an autocomplete suggestion. So it would be easiest to treat it as such, but maybe we should be smarter?

When the user selects some other section, I would guess they want the normal "replace with key" behaviour.

Set up:

Display| Name <some@e|mail

Current after typing comma:

[Display Name <some@email]||

Expect(?) after typing comma:

Display,|| mail

Origin

Based on this line https://searchfox.org/comm-central/rev/136bf46cea8bf29a481eeb53734f1c1544f8ab9f/mail/components/compose/content/addressingWidgetOverlay.js#671 for Bug 1642279

isValidAddress(input.value.substring(0, input.selectionEnd))

there was an intention to only consider the content up to the typed comma. However, when input.handleEnter is called, it still uses the full input to create a pill.

Fix

Two parts:

1. Only use the content up to the position of the typed comma to create the pill, and keep the remaining text in the address input.
2. If there is a selection, but doesn't look like an autocomplete suggestion, then we should just use the normal default behaviour.

Comment 1

[Henry Wilkes (they/them) [:henry]]

Attachment: Bug 1698484 - Tighten address pill creation on typed comma. r=aleca

Previously, pill creation would be triggered for the entire input, regardless of where the comma was typed, or whether the user had selected some of the input text.

Now, pill creation will only occur if the selection looks like an autocompletion suggestion, or if there is no selection. In the latter case, we keep the remaining input after the comma in the address input.

Comment 2

[Henry Wilkes (they/them) [:henry]]

The moz-phab link is just a work in progress draft for now, so you can see the fix.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0f6c5053ef04
Tighten address pill creation on typed comma. r=aleca

Comment 4

[newsfan]

Does that fix bug 1658157?

Comment 5

[Henry Wilkes (they/them) [:henry]]

(In reply to newsfan from comment #4)

Does that fix bug 1658157?

Not really. This just takes into account the cursor position when the comma is typed so that any pills created are prior to the comma. It also allows you to highlight a section and replace it with a single comma.

Comment 6

[Henry Wilkes (they/them) [:henry]]

Attachment: Bug 1698484 - Replace 'return' with 'break' in recipient input keydown handler. r=aleca

This tidies up revision 0f6c5053ef04.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c012f65c88b3
Replace 'return' with 'break' in recipient input keydown handler. r=aleca