560426 - Form fill interferes with custom input autocomplete arrow keys

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martijn Wargers (dead)]

To reproduce:
- go to google.com, after the page has loaded, click on the text input
- Type in a letter, for example the letter 'a'
- The formfill UI comes up, the page gets zoomed in on the text input and also the form autocomplete with suggestions from Google comes up.
- Press the arrow down key.

Expected result:
- The first autocomplete suggest entry is selected and the text input gets that value. The autocomplete 'popup' stays open and the focus stays in the text input.

Actual result:
- The first autocomplete suggest entry is selected and the text input gets that value, but soon afterwards the autocomplete 'popup' gets closed, and the focus is going to the "Google Search" button. The formfill UI is to blame for this.

This is also a problem on Wikipedia with the search.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Arrow key support was added in bug 522312. I suppose there is some way we could guess that the textbox has a fake dropdown list, but I don't think we can do it very reliably.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

This prevent using the override VK_DOWN/UP if the textbox has a autocomplete attribute to off (or similar for now, maybe the list is too much?) and has custom key listeners.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 455432 [details] [diff] [review]
Patch

>diff -r d0c7ca14452e chrome/content/browser-ui.js

>diff -r d0c7ca14452e chrome/content/forms.js

>+  _els: Cc["@mozilla.org/eventlistenerservice;1"].getService(Ci.nsIEventListenerService),
>+  _hasKeyListener: function _hasKeyListener(aElement) {
>+    let els = this._els;
>+    let listeners = els.getListenerInfoFor(aElement, {});
>+    for (let i = 0; i < listeners.length; i++) {
>+      let listener = listeners[i];
>+      if (["keyup", "keydown", "keypress"].indexOf(listener.type) != -1
>+          && !listener.inSystemEventGroup) {
>+        return true;
>+      }
>+    }
>+    return false;
>+  },

I don't like this part. Key events could be used for anything, not just fake auto-suggest UIs.

Let's drop this part.

>       case aEvent.DOM_VK_DOWN:
>-        if (currentElement instanceof HTMLTextAreaElement) {
>+        if (currentElement instanceof HTMLInputElement && !currentWrapper.canAutocomplete()) {
>+          if (this._hasKeyListener(currentElement))
>+            return;

Let's just drop the _hasKeyListener part.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 455432 [details] [diff] [review]
Patch

Change of heart. I missed the fact that the key listeners would not be called very often.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/57363b001117