1423052 - Investigate how the keyboard accessible from autocomplete would affect form autofill

Status: NEW

(Toolkit :: Form Autofill, enhancement, P3)

Description

[Ray Lin[:ralin]]

Right now we share the same keyboard accessibility with autocomplete feature, we should go through all the key handlers and review the behaviors to find the potential issue, then figure out what are desirable and what are unwanted.

Comment 1

[Ray Lin[:ralin]]

I was about to list all the keyboard shortcut here and discuss how many functionality of these should be considered as part of for form autofill as well. But now, I would like to bring up the implementation difficulty first: There's no way to get rid of running into nsFormFillController's event handler. 

For example, pressing enter is actually composed of two actions:
- event captured by nsFormFillController and run HandleEnter to fill empty string
- event captured by FormAutofillContent and trigger form autofilling according to the selected index, and populate the profile
In this case, the "empty value" hack is no harm if we remember to update the search string afterwards, and it seems able for both sides' keyhandler coexist. However, for instance when pressing "shift+delete", the selected entry is only temporarily visually deleted from suggestion list that doesn't reflect the real data in storage. And there are still other cases like this that is supposed to be refrained from in form autofill. The pain point is, on the one hand we want to totally control key events in our own code path to avoid the interference, but on the other hand we'll lose some reusable part such as navigation handling. 


I don't have very solid approach for the problem, and below is the idea I can think of now:

Since we're able to tell whether the focused input belong to form autofill, so that we can split handler into two paths, e.g.
```
case eKeyPress:
if (mAutofillInputs.Get(mFocusedInputNode)) {
  return AutofillKeyPress(aEvent);
} else {
  return KeyPress(aEvent);
}
```
so basically all we need to do is call HandleKeyNavigation if the event is derived from pressing arrow-keys.

Matt, do you think this is something we should try? Could you give me some feedback? Thanks :D

Comment 2

[Ray Lin[:ralin]]

Attachment: Bug 1423052 - (POC) Suppress certain key handlers for form autofill.

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

Comment 3

[Ray Lin[:ralin]]

Comment on attachment 8941467 [details]
Bug 1423052 - (POC) Suppress certain key handlers for form autofill.

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

Comment 4

[Ray Lin[:ralin]]

Comment on attachment 8941467 [details]
Bug 1423052 - (POC) Suppress certain key handlers for form autofill.

Hi Matt,

This patch introduces a new API to suppress some of key handlers in nsIFormFillController, which is one of approach we discussed in the meeting.  And the reason why not simply exclude all autofill fields but using flag is because in some situations we might fallback to form history[0] when should stop suppressing.

Could you give me some feedback about this patch? Thanks!



[0] https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/browser/extensions/formautofill/FormAutofillContent.jsm#113-136

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8941467 [details]
Bug 1423052 - (POC) Suppress certain key handlers for form autofill.

Clearing since Ray isn't around to work on this anymore :(