1659332 - Hitting tab quickly before address auto-completion completes leads to invalid address pill

Status: RESOLVED FIXED

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

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1659325 +++

See bug 1659325 comment #2.

Hitting tab during address entry now triggers the faulty enter behaviour (bug 1151520) here:
https://searchfox.org/comm-central/rev/444ebc51263c88a6d24f7dafdbe05a5ce79bd3d3/mail/components/compose/content/addressingWidgetOverlay.js#652

This should likely check whether a valid address was entered.

Comment 1

[Jorg K (CEST = GMT+2)]

IOW, bug 1012397 is back.

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1659332-dont-trigger-enter-on-tab.patch

What do you think guys? Another proof that meddling with auto-complete and triggering enter behaviour is not a good idea.

Comment 3

[Alessandro Castellani [:aleca]]

Comment on attachment 9170228 [details] [diff] [review]
1659332-dont-trigger-enter-on-tab.patch

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

Good thinking here, I agree we shouldn't trigger the generation of the pill if the value is not a valid address, since it might happen accidentally.
We're safe here to block the Tab since that doesn't produce a character in the field.

Unfortunately this change causes a bit of a problem.
If I type `a`, then hit TAB super quickly, the focus moves to the next field and a pill gets generated regardless.
I think we should always keep the `event.preventDefault();` if the field has any value in it.

Comment 4

[Alessandro Castellani [:aleca]]

I'll try to handle this.

Comment 5

[Alessandro Castellani [:aleca]]

Attachment: Bug 1659332 - Wait for the input timeout before firing pill creation on Tab keypress. r=ThomasD

Comment 6

[Alessandro Castellani [:aleca]]

I think this easy solution is perfect for what we need.
The TAB doesn't move the focus anywhere if the input field has anything written in it in order to avoid triggering the pill generation on blur.
After that, we check if the written input is a valid address, and if so we trigger the autocomplete enter.

It seems to work quiet nicely in order to avoid accidental pills generation of incomplete/invalid values.

Comment 7

[Alessandro Castellani [:aleca]]

Gentle ping to Thomas for this review on Phabricator.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9fb51e40644e
Wait for the input timeout before firing pill creation on Tab keypress. r=ThomasD

Comment 11

[Thomas D. (:thomas8)]

Awesome! Thanks Alex for this smart implementation!

• If Tab is pressed too fast before any results have materialized (i.e. before the general autocomplete timeout of 300ms starts to search and return results), we will now wait before calling autocomplete's handleEnter on tab exactly for the duration of the autocomplete timeout (300ms) to ensure that the search has started before we run autocomplete's handleEnter(). I understand that forceComplete will then ensure that a result is returned (the top match).

FTR, the patch has also reduced the general autocomplete timeout from 300ms to 200ms for a faster autocomplete response time (which helps, but is not required for solving the problem, see smart timeout handling above). However, except for very fast typing of 5 chars/second, this may cause even single-character searches to go to LDAP server or local search, which might have a performance backlash for big address books or slow servers. If this turns out to be a problem, we could smart up a bit more along proposals found in bug 1085674 - longer timeout for the first few characters, shorter timeout for say 3+ characters. Also note that space-separated search phrases are broken up into words, which may play a role.

Comment 12

[Brian Hauer]

For whatever it's worth, my opinion is that it's not necessary to be especially clever with the timeout period between keystrokes and lookup proposals. Obviously take this opinion with a grain of salt since I am not a Thunderbird developer; just a user.

The challenge: It's possible to type fast enough and hit tab without having ever received a proposal that tab will accept. The solution, in my opinion, is not to tune down the delay for the initial proposal and subsequent proposals while still typing an address. As you point out above, this has some potential downsides. Rather, I propose running an address resolution when the user presses tab, if no proposal has yet been offered, and automatically accept that proposal, if found. I feel the ideal solution should target working successfully even if the user types with 0ms delay between keystrokes, and adding this "on tab press" behavior does that.

(See discussions in bug #1248486.)

Comment 13

[Thomas D. (:thomas8)]

Hey Brian, sorry that my description in comment 11 was a bit ambiguous as I did not describe the details of the smart implementation to fix this bug (by ensuring that we don't try to get an autocomplete result when it's not yet there), but I was focusing on the peripheral timeout reduction and its potential problems instead. I have now added more information to my comment 11 to clarify.

So we've fixed this not mainly by reducing the timeout, but with another smart tweak along what you have postulated which ensures that autcomplete gets a chance to start searching before we handle tab, and then a result should always be returned due to forceComplete setting.

Comment 14

[Brian Hauer]

That sounds very promising! Thank you for the clarification. I'm looking forward to seeing this roll out in a future version, as it will remove a small source of frustration in my daily email usage. Thank you to everyone working on this!