Coalesce keyboard pan with zoom to element pan

VERIFIED FIXED

Status

defect
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: stechz, Assigned: vingtetun)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(firefox6 fixed, fennec6+)

Details

Attachments

(2 attachments)

The technicalities of this might be annoying to get right, but form assistant would feel soooo much better if it anticipated the keyboard coming up before transitioning to the next for element and vise versa. I promise.
Also, we need to bring the keyboard up/down after the animation has finished. Our resize takes too long.
I don't think this blocks 2.0, but certainly is something we should add. This plays into something Vivien once mentioned - about keeping the keyboard visible while navigating a form. So, once the keyboard becomes visible, we keep it visible until the users taps focus off a widget or uses the back button to close the keyboard.
tracking-fennec: --- → 2.0next+
This is not what I meant, though that would work too (might be better?). But it is key that the keyboard is not changing during our animation.
Hey I'm planning to fix bug 636339 before 2.0. The plan is to have the keyboard _always_ open while handling forms, this will allow you to navigate through list and to use the "next" key on all elements (in such a case the navigation buttons will be hidden on Android)
Assignee: nobody → 21
tracking-fennec: 2.0next+ → 6+
Posted patch PatchSplinter Review
The patch use the new Util.isKeyboardOpened method and add an event generated when the state of the keyboard change.

This prevent doing extra zooming operations when the keyboard is shown.
Attachment #533683 - Flags: review?(mark.finkle)
Comment on attachment 533683 [details] [diff] [review]
Patch

Make sure you test this on a few devices. I am worried about race conditions with the KeyboardChanged event. Your current code checks to make sure the keyboard is not open before listening for the event, but I still worry.
Attachment #533683 - Flags: review?(mark.finkle) → review+
(In reply to comment #7)
> Comment on attachment 533683 [details] [diff] [review] [review]
> Patch
> 
> Make sure you test this on a few devices. I am worried about race conditions
> with the KeyboardChanged event. Your current code checks to make sure the
> keyboard is not open before listening for the event, but I still worry.

I understand why you are worried. 
I can still add a timeout, so if the event never fires or we miss the form assistant will still react, what do you think?
Let's land it as is for now. No need to complicate the code unless we see some problems during testing.
http://hg.mozilla.org/mozilla-central/rev/e8d3246d2f2f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
I have broke the browser_autocomplete.js test with this change, I'll add a followup.
The followup ensure the form assistant has received and process the opening message before sending an autocomplete message to the form assistant :)

This also prevent doing the test on desktop where there is no such thing, otherwise the show method wait for a KeyboardChanged event that is never fired...
Attachment #534795 - Flags: review?(mark.finkle)
Comment on attachment 534795 [details] [diff] [review]
Patch - followup

>+  timer.interval(100, function() {

This makes me cry
(In reply to comment #13)
> Comment on attachment 534795 [details] [diff] [review] [review]
> Patch - followup
> 
> >+  timer.interval(100, function() {
> 
> This makes me cry

This is basically a waitFor! Maybe I should write such a method for the content side?
Attachment #534795 - Flags: review?(mark.finkle) → review+
I pushed that to try (along with some other stuff), and it seems to make Android time out even earlier, after the initial "Click on a datalist element and show suggestions" step rather than after the current "Check arrows visibility" step.
(In reply to comment #15)
> I pushed that to try (along with some other stuff), and it seems to make
> Android time out even earlier, after the initial "Click on a datalist
> element and show suggestions" step rather than after the current "Check
> arrows visibility" step.

I have simplified the test because of bug 659651, should works now.
http://hg.mozilla.org/mozilla-central/rev/74ec824bffe6
gasp, still not work like what I want on device...
Or off, since now you've got two TEST-UNEXPECTED-PASS errors on desktop ;)
Depends on: 661997
I tried to verify this bug using steps from bug 640754 which is duplicate of this bug. 

When I tap on the search field in google.com everything it's ok but when I hit back or tap outside the vkb to close it I still can see the black flicker. I see Bug 627557 is new. 
Should I close this bug or Bug 627557 as dupe of this and reopen this one?
Verified fixed on Firefox 6 Beta 2: Mozilla /5.0 (Android;Linux armv7l;rv:6.0) Gecko/20110713 Firefox/6.0 Fennec/6.0
 Mozilla /5.0 (Android;Linux armv7l;rv:6.0) Gecko/20110713 Firefox/6.0 Fennec/6.0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.