716117 - Form autocomplete popup should not show identical matches

Status: RESOLVED DUPLICATE of bug 482303

(Toolkit :: Form Manager, defect)

Description

[:Margaret Leibovic]

This is fall-out from bug 711096. On mobile, we don't show autocomplete results that exactly match what's entered in the input box, and we should do the same on desktop. Quoting Limi:

[06 15:38:41] <limi> doesn't seem to make sense to show a value that is already there :)
[06 15:38:57] <limi> that'll just mislead you into thinking you mis-typed it or something

Comment 1

[Alex Limi (:limi) — Firefox UX Team]

Bug 262206 has the rationale for the original behavior. Reasons why I think our current behavior on desktop is wrong:

* Showing an entry that does nothing seems wrong to me.
* Potentially misleading in the other direction; "did I mis-type that?".
* I don't think the "delete the exact value I pasted in" or "have I already typed this before — yes, I have" indicator are compelling enough to keep this behavior.

Of course, these things are a matter of opinion, and this is mine. Bikeshed hat ON! ;)

Comment 2

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Do we not already behave like this? (looking at the code it seems like we don't but when I actually try it, I don't see matches for my exact string).

Bug 262206 looks to be about the location bar. This is about normal form fields right?

Comment 3

[:Margaret Leibovic]

(In reply to Paul O'Shannessy [:zpao] from comment #2)
> Bug 262206 looks to be about the location bar. This is about normal form
> fields right?

Yeah, this is about normal form fields.

Comment 4

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to Paul O'Shannessy [:zpao] from comment #2)
> Do we not already behave like this? (looking at the code it seems like we
> don't but when I actually try it, I don't see matches for my exact string).

Oh ha! That's because I tried to fix it before actually testing and I built it without realizing. So, patch coming, but I don't really want to write tests... volunteers? (Felix?) I haven't checked on the async patch recently, so this might need to be integrated there if it lands first.

Comment 5

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1 (needs tests)

Matt, simple enough?

Comment 6

[Matthew N. [:MattN]]

Comment on attachment 586623 [details] [diff] [review]
Patch v0.1 (needs tests)

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

(In reply to Alex Limi (:limi) — Firefox UX Team from comment #1)
> * Potentially misleading in the other direction; "did I mis-type that?".

I don't see how this would be the case since you wouldn't normally see the autocomplete for a typo.  I'm on the side that thinks that autocomplete confirms that I didn't make a typo and I think that making it harder to delete an entry (which is already non-obvious) isn't a win.

I regards to deleting entries, this patch makes it difficult to delete one character autocomplete entries since typing that one character will not show it to be able to delete it.  The only way would be to hit the down arrow without typing anything and going through every entry sorted by frecency to find the single character entry to delete. If there's a limit on the number of results shown then it would be impossible to delete them through the UI.  I don't remember if we limit in FormAC or Autocomplete though, and of course single character autocomplete entries are not useful other than to validate that text was entered before. (Think of a website that uses separate inputs for each character such as a zip code.)

(In reply to Paul O'Shannessy [:zpao] from comment #2)
> Bug 262206 looks to be about the location bar. This is about normal form
> fields right?

It was in the autocomplete components which would affect the location bar, form manager and password manager.  I'm pretty sure we don't want this fix for password manager since in that case clicking on the entry in the dropdown will insert the password.  (This is most common if a user has more than one password saved so we don't fill in either of them by default).

f+ on the code but another won't fix would work for me too.  And yes, it would probably be best to make this change on async formAC instead since it will replace the sync version.

::: toolkit/components/satchel/nsFormAutoComplete.js
@@ +316,5 @@
>                      frecency:       stmt.row.frecency,
>                      totalScore:     Math.round(stmt.row.frecency * stmt.row.boundaryBonuses)
>                  }
> +                if (entry.text == searchString)
> +                    continue;

It would be marginally more efficient to move the test to the beginning of the while loop.