701947 - VKB does not appear when tapping into an iframe text box

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

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)]

Can't type in it at all.

Comment 2

[Alex Pakhotin (:alexp)]

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)]

(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)]

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)]

Attachment: Fix

Added a check for designMode=="on".

Comment 6

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

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)]

(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)]

Attachment: 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?)]

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)]

http://hg.mozilla.org/projects/birch/rev/5f5f3bb411ff

Comment 11

[Aaron Train [:aaronmt]]

Samsung Nexus S (Android 2.3.6)
20111122060933
http://hg.mozilla.org/projects/birch/rev/c26b7a14e5bd

Comment 12

[Martijn Wargers (dead)]

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.