Recipient autocomplete suggestion overrides ANY manual address input if quickly entered/pasted and confirmed with Enter/Tab before autocomplete suggestions disappear

RESOLVED FIXED in Firefox 42

Status

()

defect
--
critical
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: magnusbae, Assigned: mkmelin)

Tracking

({dataloss, privacy})

unspecified
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.111 Safari/537.36

Steps to reproduce:

- Start typing in an address, wait for autocomplete
- Note that autocomplete does not suggest the address you want
- Quickly type in the rest of the address you are e-mailing and hit enter before the autocomplete suggestions disappear


Actual results:

- Recipient row contains the first of the autocomplete suggestions provided earlier.


Expected results:

- Recipient row should contain the address I just typed in.
Magnus, thanks for reporting!
Grrrrr... autocomplete is really giving us a headache...

Magnus, is this covered by some other bug? I suspect not, because it's like the other way round from the complaints we had so far:

Here: Autocomplete is slow to disappear, thus does not KEEP my actual input
Other bugs: Autocomplete is slow to appear, thus does not CHANGE (autocomplete) my actual input
Hmm, this check is probably what's causing it, and likely redundant now.
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/autocomplete/nsAutoCompleteController.cpp#1352
Assignee: nobody → mkmelin+mozilla
OS: Windows 8.1 → All
Hardware: x86 → All
Sigh, I'm starting to hate bugs where autocomplete randomly swaps recipients against users intention...
Which is datalossy and bad violation of privacy -> critical

Magnus, you seemed to have a good idea of how to fix this when you assigned this bug to yourself:
(In reply to Magnus Melin from comment #2)
> Hmm, this check is probably what's causing it, and likely redundant now.
> http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/
> autocomplete/nsAutoCompleteController.cpp#1352
Severity: normal → critical
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(mkmelin+mozilla)
Keywords: dataloss, privacy
What this really means is that ANY *rapidly* confirmed changes/additions/replacements of a given autocomplete suggestion will FAIL without exception - this should be fixed asap.

Sample scenario:

user types "jo" to see if john.miller@asdf.com is already in AB
autocomplete suggests "john.miller@bar.com"
user selects "bar.com", pastes in "asdf.com", and immediately confirms with tab or Enter.
TB cunningly reverts that to the original autocomplete suggestion, "john.miller@bar.com".
Which might be an entirely different person.

Even replacing the entire address will be ignored if done quickly:
Let's say autocomplete suggested "joe@bar.com" which is an entirely unrelated contact;
user selects all of that with Ctrl+A, pastes in "john.miller@asdf.com", and immediately confirms with tab or Enter.
TB cunningly reverts that to the original autocomplete suggestion, "joe@bar.com".

Confidential data can easily get sent to the wrong person, which is a massive violation of privacy.
It's not always easy to notice that TB has swapped the recipient; worse, there's absolutely no reason to recheck if you know what you've just entered is correct, and you've confirmed that.

Power user's natural reaction: TB sucks!
We're talking of those power users who we need to recommend Thunderbird to their employees, workgroups, etc.
Summary: Autocomplete suggestions override typed in value when pressing enter before autocomplete suggestions disappear → Recipient autocomplete suggestion overrides ANY manual address input if quickly entered/pasted and confirmed with Enter/Tab before autocomplete suggestions disappear
Duplicate of this bug: 1160861
There's an impressive variety of scenarios where TB reverts user's explicitly confirmed recipient and cunningly picks another unrelated recipient instead (which is dataloss and privacy violation):

Bug 1130858 - Recipient autocomplete suggestion overrides ANY manual address input if quickly entered/pasted and confirmed with Enter/Tab before autocomplete suggestions disappear

Bug 1138033 - Stubborn recipient autocomplete silently swaps recipients: Cannot compose message to valid, normal, new email address (doe@asdf.com/admin@foo.co) if similar longer address already exists in AB (john.doe@asdf.com/admin@foo.com)

Bug 1152517 - Recipient autocomplete wrongly considers last mouse-hovered contact from results dropdown "selected" and then uses that unintended, random recipient upon blur (via Tab, Enter, or when moving to subject or body)

So to work around all of them, users will need to:
- Slow down when entering recipients to avoid Bug 1130858
- Put a comma even after a single recipient, or add every recipient to your AB first, to avoid Bug 1138033
- Move their mouse pointer into some safe corner of the screen before(!) entering a recipient, to avoid Bug 1152517

Easy, ain't it? :p
See Also: → 1138033, 1152517
See Also: → 1012397
We can get rid of the if (value.IsEmpty()) use default case. 

We now go through the matches and find the one that case-insensitively matches. If there's no match there's no match. If there was a match it would have been found in the look and the value set. So it must be something not matching, and we should not change the value.

The codepath in question is only executed for forceComplete=true, i.e. thunderbird. Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c9e749760b09
Flags: needinfo?(mkmelin+mozilla)
Attachment #8622630 - Flags: review?(mak77)
Component: Message Compose Window → Autocomplete
Product: Thunderbird → Toolkit
Version: 31 → unspecified
note I will be traveling tomorrow, then in Whistler, the review turn around time might not be the nicest.
Comment on attachment 8622630 [details] [diff] [review]
bug1130858_autocomplete_overrides_entered.patch

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

Off-hand the change makes sense.  I'm sort of blind flying here since you are the only forcecomplete consumer, so you likely know better than me how it should behave.

I think sooner or later you should spend resources on writing some tests for toolkit/autocomplete, the situation doesn't look good, the risk of breaking TB unwillingly is high and you have no test coverage for these options...
Attachment #8622630 - Flags: review?(mak77) → review+
We should be able to get this into TB 38.1.0 via THUNDERBIRD_38_VERBRANCH but it should really land in core first.
I think it's better to hold of and let it bake for until the next point release.
Needed to fix a test. But, what the was testing something unrealistic so... 

completeSelectedIndex doesn't set the textValue when there's suggestions
Attachment #8631770 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/d42f9b90ecad
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Inclusion into TB 38.x: See bug 1187519.
Blocks: 1187519
You need to log in before you can comment on or make changes to this bug.