Last Comment Bug 701947 - VKB does not appear when tapping into an iframe text box
: VKB does not appear when tapping into an iframe text box
Status: VERIFIED FIXED
[testday-20111111], [VKB]
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: P2 normal (vote)
: ---
Assigned To: Alex Pakhotin (:alexp)
:
:
Mentors:
http://people.mozilla.com/~nhirata/ht...
Depends on: native_droid_gesture
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-11 16:52 PST by Naoki Hirata :nhirata (please use needinfo instead of cc)
Modified: 2016-07-29 14:20 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
Fix (1.18 KB, patch)
2011-11-18 18:03 PST, Alex Pakhotin (:alexp)
no flags Details | Diff | Splinter Review
Fix v2 (1.58 KB, patch)
2011-11-21 19:24 PST, Alex Pakhotin (:alexp)
mark.finkle: review+
Details | Diff | Splinter Review

Description Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-11-11 16:52:03 PST
1. goto http://people.mozilla.com/~nhirata/html_tp/bug573447.htm
2. click in the iframe in the middle

Expected: VKB appears
Actual: nothing happens.

Testday build
Comment 1 Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-11-11 16:52:36 PST
Can't type in it at all.
Comment 2 Alex Pakhotin (:alexp) 2011-11-14 17:08:24 PST
Regression with the latest pan/zoom layers implementation - it used to work before.
Patrick, do you have a clue why this has broken?
Comment 3 Patrick Walton (:pcwalton) 2011-11-14 17:10:03 PST
(In reply to Alex Pakhotin (:alexp) from comment #2)
> Regression with the latest pan/zoom layers implementation - it used to work
> before.
> Patrick, do you have a clue why this has broken?

Not off the top of my head. I'll need to investigate.
Comment 4 Alex Pakhotin (:alexp) 2011-11-17 19:05:20 PST
This issue and bug 701706 are caused by the new check in the BrowserEventHandler.handleEvent() function: http://hg.mozilla.org/projects/birch/file/04b6e59f4e82/mobile/android/chrome/content/browser.js#l1300
It calls _elementReceivesInput() function, which returns false for the editable iframe element, and for the area outside of the edit boxes, so the "mousedown" event is not handled properly.
I'm not sure at the moment how to fix it, but some better check is definitely needed there.

Actually, I don't really see any issues if _elementReceivesInput() just always returns true, or its call (with aEvent.preventDefault) is just removed. What kind of selection is it supposed to stop?
Comment 5 Alex Pakhotin (:alexp) 2011-11-18 18:03:11 PST
Created attachment 575611 [details] [diff] [review]
Fix

Added a check for designMode=="on".
Comment 6 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-18 19:29:32 PST
Comment on attachment 575611 [details] [diff] [review]
Fix

This is good, but you might be missing a few cases that we had covered in XUL Fennec. See if you can add any more checks from here:
http://mxr.mozilla.org/mozilla-central/source/mobile/xul/chrome/content/forms.js#536
Comment 7 Alex Pakhotin (:alexp) 2011-11-21 09:46:48 PST
(In reply to Mark Finkle (:mfinkle) from comment #6)
> 
> This is good, but you might be missing a few cases that we had covered in
> XUL Fennec. See if you can add any more checks from here:
> http://mxr.mozilla.org/mozilla-central/source/mobile/xul/chrome/content/
> forms.js#536

Yes, I checked that, just wasn't sure if the other cases aren't already covered by the current implementation - it's a bit different approach used here in the browser.js compared to forms.js.
But probably I'll just copy the _isEditable function from the forms and call it instead of my one additional check.
Comment 8 Alex Pakhotin (:alexp) 2011-11-21 19:24:04 PST
Created attachment 576071 [details] [diff] [review]
Fix v2

Using the _isEditable() function from the forms.js to have a more thorough check for an editable element.

Though it fixes the bug, the workaround patch from bug 701706 would make the _elementReceivesInput() function obsolete. I guess we have to decide, which way to choose to fix these regressions.
Comment 9 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-21 19:33:30 PST
Comment on attachment 576071 [details] [diff] [review]
Fix v2

This will have to do for know. Fixing these issues by better controlling the mouse events is a longer term solution.
Comment 10 Alex Pakhotin (:alexp) 2011-11-21 21:17:16 PST
http://hg.mozilla.org/projects/birch/rev/5f5f3bb411ff
Comment 11 Aaron Train [:aaronmt] 2011-11-22 07:12:40 PST
Samsung Nexus S (Android 2.3.6)
20111122060933
http://hg.mozilla.org/projects/birch/rev/c26b7a14e5bd
Comment 12 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-12-16 09:05:00 PST
Actually, I can still reproduce this bug in current trunk build, I guess it might be a new regression, so I filed bug 711496 for it.

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