636339 - Better VKB integration for form assistant

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, enhancement, P4)

Description

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

Actually the keyboard opens and close while navigating through a form and this has a few bad sides effect:
 * the next/previous buttons position can change
 * the content area is zoomed/unzoomed and the content is repositionned
 * this prevent the user to use his keyboard for navigating thought list element
 * this do not exploit the "->" key of some keyboard (or event "Tab")

Comment 1

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

The other approach discussed was to let the VKB close, but make the [next][prev] buttons re-appear in that situation.

Comment 2

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

(In reply to comment #1)
> The other approach discussed was to let the VKB close, but make the
> [next][prev] buttons re-appear in that situation.

I guess there isn't a good solutions except this one on landscape.

Comment 4

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

Attachment: WIP v0.1

Comment 5

[Madhava Enros [:madhava]]

I've been thinking about how to reconcile the differences between our form assistant and the Android standard behavior.

Generally, moving to using the keyboard "next" button, when the keyboard is visible, makes sense. The complication is what happens when the user is using an on-screen element that doesn't keep the keyboard on the screen, like a listbox.

Keeping our existing [ up arrow | down arrow ] buttons on screen in those cases feels too... cobbled together, I think.  It means that sometimes you're using android-standard controls, and in other cases, you're looking for altogether different controls.

I'd be tempted, for a v1, just to have parity with the Android approach - i.e. use the keyboard when available, and when the keyboard's not there, you don't get a form assistant controls. This actually lessens our current breadth of functionality, but is simple and consistent with Android.  From that starting point, we could start looking at ways to make it smarter again.

Comment 6

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

(In reply to comment #5)
> I've been thinking about how to reconcile the differences between our form
> assistant and the Android standard behavior.
> 
> Generally, moving to using the keyboard "next" button, when the keyboard is
> visible, makes sense. The complication is what happens when the user is
> using an on-screen element that doesn't keep the keyboard on the screen,
> like a listbox.
> 
> Keeping our existing [ up arrow | down arrow ] buttons on screen in those
> cases feels too... cobbled together, I think.  It means that sometimes
> you're using android-standard controls, and in other cases, you're looking
> for altogether different controls.
> 
> I'd be tempted, for a v1, just to have parity with the Android approach -
> i.e. use the keyboard when available, and when the keyboard's not there, you
> don't get a form assistant controls. This actually lessens our current
> breadth of functionality, but is simple and consistent with Android.  From
> that starting point, we could start looking at ways to make it smarter again.

I will implement something like that before the end of the week,there are some issues with keeping the keyboard opened that can't be fixed for the 6.0 release.

Comment 7

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

Attachment: Patch

No arrows on Android

Comment 8

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

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

># HG changeset patch

>+#ifndef ANDROID
>     // Update the next/previous commands
>     this._cmdPrevious.setAttribute("disabled", !aHasPrevious);
>     this._cmdNext.setAttribute("disabled", !aHasNext);
> 
>     // If both next and previous are disabled don't bother showing arrows
>-    if (!aHasNext && !aHasPrevious)
>+    if (aHasNext || aHasPrevious)
>+      this._container.removeAttribute("disabled");
>+    else
>+#endif
>       this._container.setAttribute("disabled", "true");

Can we use an #else to make the code more clear?

>+#ifndef ANDROID
>     // Update the next/previous commands
>     this._cmdPrevious.setAttribute("disabled", !aHasPrevious);
>     this._cmdNext.setAttribute("disabled", !aHasNext);
> 
>     // If both next and previous are disabled don't bother showing arrows
>-    if (!aHasNext && !aHasPrevious)
>+    if (aHasNext || aHasPrevious)
>+      this._container.removeAttribute("disabled");
>+    else
>       this._container.setAttribute("disabled", "true");
>+#else
>+    // Always hide the arrows on Android
>+    this._container.setAttribute("disabled", "true");
>+#endif


r+ with that tweak

Comment 9

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

http://hg.mozilla.org/mozilla-central/rev/313e897eb38d

Comment 10

[Anna (Waverley)]

Since the form assistant feature was removed I will mark this as 

VERIFIED FIXED on:

Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:7.0a1) Gecko/20110525 Firefox/7.0a1 Fennec/7.0a1 

Device: HTC Desire Z (Android 2.2)