1429718 - Suggestion result is cached if the search string is identical to the autofilled value

Status: RESOLVED FIXED

(Toolkit :: Form Autofill, defect, P3)

Description

[Ray Lin[:ralin]]

STR:
1. create one new address profile with organization name: "test"
2. visit https://luke-chang.github.io/autofill-demo/basic.html
3. focus organization field and enter "test"
4. press arrow-down and then press enter to auto-fill profile
5. press arrow-down again

Expected result:
clear form button dropdown shows up

Actual result:
see the previous suggestion result againd

Comment 1

[Ray Lin[:ralin]]

Hi Marco,

In some cases, we will need to manually invalidate the cached result even the search string was never changed, and I found an exposed API called ResetInternalState in AutocompleteController that might be useful for us. 

Do you know is there any concern using this API? I'm thinking to call it every time after startSearch() to prevent our result from being cached. 

Thanks :D

Comment 2

[Marco Bonardo [:mak]]

The scope of ResetInternalState() is to reset the controller when the input field didn't change from the previous search, but results are no more relevant to the current context. For example the Address bar is the same for all the tabs, but when switching tabs we don't want results from the previous tab to pollute a new search in the new tab. Previously we used to detach/attach controller, but that was more expensive.
You can use it, it will pretty much be like saying "this is a completely new search". Or you could modify your frontend code to handle DOWN differently in this case. It depends on your code.

Comment 3

[Ray Lin[:ralin]]

Thank you so much for the useful information!

(In reply to Marco Bonardo [::mak] from comment #2)
> The scope of ResetInternalState() is to reset the controller when the input
> field didn't change from the previous search, but results are no more
> relevant to the current context. 
I think that's what similar to our usage for form autofill. The internal context changed after fields being populated, and should refresh the result no matter the previous search string.

> You can use it, it will pretty much be like saying "this is a completely new
> search". Or you could modify your frontend code to handle DOWN differently
> in this case. It depends on your code.
I'll try ResetInternalState first to see if it can fit our need, and investigate our own key handling later. Thanks again for your help :D

Comment 4

[Ray Lin[:ralin]]

Attachment: Bug 1429718 - Reset AutocompleteController state every startSearch for formautofill to prevent any result being cached.

Review commit: https://reviewboard.mozilla.org/r/213138/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/213138/

Comment 5

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8942875 [details]
Bug 1429718 - Reset AutocompleteController state every startSearch for formautofill to prevent any result being cached.

https://reviewboard.mozilla.org/r/213138/#review219216

::: browser/extensions/formautofill/FormAutofillContent.jsm:168
(Diff revision 1)
> +    }
> +
> +    Promise.resolve(pendingSearchResult).then((result) => {
>        listener.onSearchResult(this, result);
>        ProfileAutocomplete.lastProfileAutoCompleteResult = result;
> +      autocompleteController.resetInternalState();

Would be better to have a comment for explaining what it's for.

Comment 6

[Ray Lin[:ralin]]

Comment on attachment 8942875 [details]
Bug 1429718 - Reset AutocompleteController state every startSearch for formautofill to prevent any result being cached.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/213138/diff/1-2/

Comment 7

[Ray Lin[:ralin]]

Thanks for reviewing, I've added a comment above the line :D

TH result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=05616390a4223d4449cf0e74dceaaf3d58d433fe

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0fd485f3f388
Reset AutocompleteController state every startSearch for formautofill to prevent any result being cached. r=lchang

Comment 9

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/0fd485f3f388