Closed Bug 1429718 Opened 2 years ago Closed 2 years ago

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

Categories

(Toolkit :: Form Autofill, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: ralin, Assigned: ralin)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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
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
Flags: needinfo?(mak77)
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.
Flags: needinfo?(mak77)
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
Status: NEW → ASSIGNED
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.
Attachment #8942875 - Flags: review?(lchang) → review+
Thanks for reviewing, I've added a comment above the line :D

TH result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=05616390a4223d4449cf0e74dceaaf3d58d433fe
Pushed by ralin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0fd485f3f388
Reset AutocompleteController state every startSearch for formautofill to prevent any result being cached. r=lchang
https://hg.mozilla.org/mozilla-central/rev/0fd485f3f388
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.