Last Comment Bug 775403 - Focus (?) not working for tabs in Gaia browser app (virtual keyboard broken for remote content in browser)
: Focus (?) not working for tabs in Gaia browser app (virtual keyboard broken f...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: :Felipe Gomes (needinfo me!)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 779358
Blocks: 819102 b2g-e10s-work 774139
  Show dependency treegraph
 
Reported: 2012-07-18 21:22 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2014-03-12 05:30 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Experiment (3.96 KB, patch)
2012-07-18 22:41 PDT, :Felipe Gomes (needinfo me!)
cjones.bugs: review+
cjones.bugs: feedback+
Details | Diff | Splinter Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-18 21:22:22 PDT
This is almost certainly due to bug 774139.

Testing on http://people.mozilla.com/~cjones/events-mouse.html, which doesn't preventDefault() the touchstart event so should only get mouse events IIUC, I see

received event touchstart
received event mousedown
received event touchmove   <== wrong, I think
received event mousemove
received event touchmove
received event mousemove
...
received event touchend    <== also wrong, I think, maybe not
received event mouseup

This is probably confusing either our focus code or the JS form helper.
Comment 1 :Felipe Gomes (needinfo me!) 2012-07-18 21:41:45 PDT
That sequence seems right to me. If you have listeners, you'll always get the touch events. And if the code called preventDefault() then it should stop getting mouse events, but will still get touch.
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-18 21:44:43 PDT
OK.  Good to know, my understanding was incorrect.
Comment 3 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-18 22:08:19 PDT
Alright, doesn't seem to be touch-event related.  Whew!

If I load this page

http://people.mozilla.com/~cjones/focus.html

as the "Template" app, set to run out-of-process, it brings up the VKB just fine.  If I remove the .focus() call and tap the app, it works just fine too.

However, if I load the same page in the browser app, it *doesn't* work, whether with explicit .focus() or tapping the input box.

This smells to me like a missing docShell.activate on the content side.  Dale, Justin, can you guys investigate?
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-18 22:09:23 PDT
Another possibility is that this is a recent bug in the gaia browser app.  If so please close out.
Comment 5 :Felipe Gomes (needinfo me!) 2012-07-18 22:41:21 PDT
Created attachment 643728 [details] [diff] [review]
Experiment
Comment 6 :Felipe Gomes (needinfo me!) 2012-07-18 22:56:27 PDT
Here's what I think is happening:

You remember when you were dealing with bug 761927 and debugging through nsFocusManager::SetFocusInner et. al?  The focus manager is activated by mouse or keyboard input events.

Now, in bug 774139, we stopped sending mouse events to the parent element, in favor of creating the phony mouse events in the content.  But this means that SetFocusInner won't get called and remote->Activate() won't be called (http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#1781)

The solution is that we can still block cross-process forwarding of mouse events in that case, but we make them reach the parent frame as well. This seems a more correct behavior.

Another hack solution would be to call remote->Activate()  in nsEventStateManager::HandleCrossProcessEvent, but that feels like a hack and I believe can fall into other focus issues.
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-18 23:01:57 PDT
Comment on attachment 643728 [details] [diff] [review]
Experiment

Fixes the bug for me.
Comment 8 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-18 23:03:09 PDT
Indeed a regression from forwarding touch events.
Comment 9 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-18 23:13:04 PDT
A potential problem with this patch is that IIUC, it will result in synthesized mouse events *always* being delivered to in-process content, even if the content preventDefault().  I don't think that's all that serious of a problem but we'd need to test.
Comment 10 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-19 12:36:35 PDT
Comment on attachment 643728 [details] [diff] [review]
Experiment

smaug is OK with delegating this review to me.
Comment 11 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-19 12:40:56 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5f8ebf59bbf
Comment 12 Ed Morley [:emorley] 2012-07-20 06:44:51 PDT
https://hg.mozilla.org/mozilla-central/rev/f5f8ebf59bbf

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