Last Comment Bug 636339 - Better VKB integration for form assistant
: Better VKB integration for form assistant
Status: VERIFIED FIXED
[vkb]
:
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: All All
: P4 enhancement (vote)
: ---
Assigned To: Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
:
Mentors:
: 650608 (view as bug list)
Depends on: 630576
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-23 17:15 PST by Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
Modified: 2011-07-14 21:40 PDT (History)
9 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
WIP v0.1 (5.58 KB, patch)
2011-05-03 10:10 PDT, Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
no flags Details | Diff | Review
Patch (997 bytes, patch)
2011-05-19 04:08 PDT, Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
mark.finkle: review+
Details | Diff | Review

Description Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2011-02-23 17:15:15 PST
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?) 2011-04-13 13:26:59 PDT
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) 2011-04-13 15:27:00 PDT
(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 3 Mark Finkle (:mfinkle) (use needinfo?) 2011-04-17 07:47:05 PDT
*** Bug 650608 has been marked as a duplicate of this bug. ***
Comment 4 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2011-05-03 10:10:51 PDT
Created attachment 529741 [details] [diff] [review]
WIP v0.1
Comment 5 Madhava Enros [:madhava] 2011-05-18 09:25:41 PDT
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) 2011-05-18 09:27:53 PDT
(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) 2011-05-19 04:08:52 PDT
Created attachment 533589 [details] [diff] [review]
Patch

No arrows on Android
Comment 8 Mark Finkle (:mfinkle) (use needinfo?) 2011-05-19 05:53:32 PDT
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) 2011-05-20 03:20:43 PDT
http://hg.mozilla.org/mozilla-central/rev/313e897eb38d
Comment 10 Anna (Waverley) 2011-05-26 02:24:40 PDT
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)

Note You need to log in before you can comment on or make changes to this bug.