Android editors have a unique set of key bindings, as implemented through the MovementMethod interface [1]. For example, Ctrl+Left moves cursor to start of line, Ctrl+Right moves cursor to end of line, Alt+Left moves cursor to previous word, Alt+Right moves cursor to next word Right now we seem to use Emacs-like bindings [2] because we don't have platform-specific bindings for Android [3], but it should be straightforward to add a set of custom bindings for Android. This would make text input experience in Fennec more in line with the rest of Android (especially on devices with hardware keyboards), and is also necessary to fix bugs (e.g. Bug 706336) which assume the Android bindings. [1] http://developer.android.com/reference/android/text/method/MovementMethod.html [2] http://mxr.mozilla.org/mozilla-central/source/content/xbl/builtin/emacs/platformHTMLBindings.xml [3] http://mxr.mozilla.org/mozilla-central/source/content/xbl/builtin/Makefile.in

I think this is worth fixing now with quite a few people requesting this feature.

Can you review this, Neil? I saw your name as a peer for /content/xbl/builtin I started out using /content/xbl/builtin/unix as an example, and gathered navigation key bindings from [1] and delete key bindings from [2]. [1] http://androidxref.com/4.2_r1/xref/frameworks/base/core/java/android/text/method/BaseMovementMethod.java [2] http://androidxref.com/4.2_r1/xref/frameworks/base/core/java/android/text/method/BaseKeyListener.java

These changes make us able to pass combinations such as Ctrl+A to Gecko.

(In reply to Jim Chen from comment #3) > I started out using /content/xbl/builtin/unix as an example Would you mind using hg cp to show that the files were based on existing files?

Comment on attachment 717307 [details] [diff] [review] Add Android XBL key bindings (v1) > ifneq (,$(filter OS2 WINNT,$(OS_ARCH))) > DIRS = win > else > ifeq (cocoa,$(MOZ_WIDGET_TOOLKIT)) > DIRS = mac > else > ifneq (,$(filter qt gtk2,$(MOZ_WIDGET_TOOLKIT))) > DIRS = unix > else >+ifeq (android,$(MOZ_WIDGET_TOOLKIT)) >+DIRS = android >+else > DIRS = emacs > endif > endif > endif >+endif The placement is confusing, could you put your check outside the unix/emacs check please?

Thanks for the quick response Neil! (In reply to neil@parkwaycc.co.uk from comment #5) > (In reply to Jim Chen from comment #3) > > I started out using /content/xbl/builtin/unix as an example > Would you mind using hg cp to show that the files were based on existing > files? Okay. I'm on git right now but I'll do that when I move this to hg. (In reply to neil@parkwaycc.co.uk from comment #6) > Comment on attachment 717307 [details] [diff] [review] > Add Android XBL key bindings (v1) > > > ifneq (,$(filter OS2 WINNT,$(OS_ARCH))) > > DIRS = win > > else > > ifeq (cocoa,$(MOZ_WIDGET_TOOLKIT)) > > DIRS = mac > > else > > ifneq (,$(filter qt gtk2,$(MOZ_WIDGET_TOOLKIT))) > > DIRS = unix > > else > >+ifeq (android,$(MOZ_WIDGET_TOOLKIT)) > >+DIRS = android > >+else > > DIRS = emacs > > endif > > endif > > endif > >+endif > The placement is confusing, could you put your check outside the unix/emacs > check please? So like this? > ifneq (,$(filter OS2 WINNT,$(OS_ARCH))) > DIRS = win > else > ifeq (cocoa,$(MOZ_WIDGET_TOOLKIT)) > DIRS = mac > else > +ifeq (android,$(MOZ_WIDGET_TOOLKIT)) > +DIRS = android > +else > ifneq (,$(filter qt gtk2,$(MOZ_WIDGET_TOOLKIT))) > DIRS = unix > else > DIRS = emacs > endif > endif > endif > +endif

(In reply to Jim Chen from comment #7) > I'm on git right now but I'll do that when I move this to hg. I've never used git but hg produces git-style diffs so surely git must have a way to show that content/xbl/builtin/android/jar.mn is a copy of content/xbl/builtin/unix/jar.mn?

Comment on attachment 717309 [details] [diff] [review] Properly pass meta states to Gecko (v1) Review of attachment 717309 [details] [diff] [review]: ----------------------------------------------------------------- LGTM!

(In reply to neil@parkwaycc.co.uk from comment #8) > (In reply to Jim Chen from comment #7) > > I'm on git right now but I'll do that when I move this to hg. > I've never used git but hg produces git-style diffs so surely git must have > a way to show that content/xbl/builtin/android/jar.mn is a copy of > content/xbl/builtin/unix/jar.mn? According to [1], git does not track copies but instead tries to detect copies (and it does a rather poor job at it when I tried). I went ahead and moved this to hg. Right now android/platformHTMLBindings.xml is sufficiently different from unix/platformHTMLBindings.xml that I kept it a new file instead of a copy; this way the diff doesn't show deleting all the unix bindings, which I think is more confusing than helpful. [1] http://stackoverflow.com/questions/1043388/record-file-copy-operation-with-git

(In reply to Jim Chen from comment #10) > According to [1], git does not track copies but instead tries to detect > copies (and it does a rather poor job at it when I tried). > > I went ahead and moved this to hg. Right now > android/platformHTMLBindings.xml is sufficiently different from > unix/platformHTMLBindings.xml that I kept it a new file instead of a copy; > this way the diff doesn't show deleting all the unix bindings, which I think > is more confusing than helpful. Fair enough. (I did wonder whether the windows bindings looked closer to the android bindings, but I think you would still needed to have delete some of the bindings, which would make the result confusing anyway.) (In reply to Jim Chen from comment #7) > So like this? > > ifneq (,$(filter OS2 WINNT,$(OS_ARCH))) > > DIRS = win > > else > > ifeq (cocoa,$(MOZ_WIDGET_TOOLKIT)) > > DIRS = mac > > else > > +ifeq (android,$(MOZ_WIDGET_TOOLKIT)) > > +DIRS = android > > +else > > ifneq (,$(filter qt gtk2,$(MOZ_WIDGET_TOOLKIT))) > > DIRS = unix > > else > > DIRS = emacs > > endif > > endif > > endif > > +endif Yes, except the extra endif appears to have got lost in the move to hg :-(

(In reply to neil@parkwaycc.co.uk from comment #11) > > > DIRS = emacs > > > endif > > > endif > > > endif > > > +endif > Yes, except the extra endif appears to have got lost in the move to hg :-( Oops. Good catch! :)

Comment on attachment 717424 [details] [diff] [review] Add Android XBL key bindings (v4) Huh, I wonder why cmd_selectChar/LinePrevious/Next aren't in browser-base.inc; I should file a bug on that.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3f6e6912e71a https://hg.mozilla.org/integration/mozilla-inbound/rev/77a94dc888c9

Backed out for Android mochitest failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/373699ce4c96 https://tbpl.mozilla.org/php/getParsedLog.php?id=20071168&tree=Mozilla-Inbound 6647 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | input event must be bubbles 6648 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | perhaps, timestamp wasn't set correctly :Mon Feb 25 10:05:21 2013 6649 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | Editor1, body has contenteditable attribute: input event wasn't fired by Undo 6650 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | Editor1, body has contenteditable attribute: input event by Undo wasn't trusted event 6651 ERROR TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | Editor1, body has contenteditable attribute: input event wasn't fired by Redo 6652 ERROR TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | uncaught exception - TypeError: inputEvent is null at http://mochi.test:8888/tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html:148 6653 INFO TEST-END | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | finished in 1345ms 10606 INFO TEST-PASS | /tests/editor/libeditor/text/tests/test_dom_input_event_on_texteditor.html | perhaps, timestamp wasn't set correctly :Mon Feb 25 10:05:48 2013 10607 INFO TEST-PASS | /tests/editor/libeditor/text/tests/test_dom_input_event_on_texteditor.html | <input type="text">: Accel+Z key didn't undo the value 10608 INFO TEST-PASS | /tests/editor/libeditor/text/tests/test_dom_input_event_on_texteditor.html | <input type="text">: input event wasn't fired by Undo 10609 INFO TEST-PASS | /tests/editor/libeditor/text/tests/test_dom_input_event_on_texteditor.html | <input type="text">: input event by Undo wasn't trusted event 10610 ERROR TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/text/tests/test_dom_input_event_on_texteditor.html | <input type="text">: Accel+Y key didn't redo the value - got , expected 10611 ERROR TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/text/tests/test_dom_input_event_on_texteditor.html | <input type="text">: input event wasn't fired by Redo 10612 ERROR TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/text/tests/test_dom_input_event_on_texteditor.html | uncaught exception - TypeError: inputEvent is null at http://mochi.test:8888/tests/editor/libeditor/text/tests/test_dom_input_event_on_texteditor.html:119 10613 INFO TEST-END | /tests/editor/libeditor/text/tests/test_dom_input_event_on_texteditor.html | finished in 839ms

(In reply to Ryan VanderMeulen [:RyanVM] from comment #15) > /tests/editor/libeditor/text/tests/test_dom_input_event_on_texteditor.html | > <input type="text">: Accel+Y key didn't redo the value - got , expected Looks like the test uses Accel+Y for redo, which wouldn't work on Android with the new bindings. Masayuki-san: can test_dom_input_event_on_texteditor.html and test_dom_input_event_on_htmleditor.html use Shift+Accel+Z for redo for all platforms? Shift+Accel+Z works on all platforms [1], but Accel+Y only works on some platforms [2]. If we change to Shift+Accel+Z for all platforms, we don't have to detect the platform, and I don't think the change will impact the test's functionality. [1] http://mxr.mozilla.org/mozilla-central/source/content/xbl/builtin/input-fields-base.inc#12 [2] http://mxr.mozilla.org/mozilla-central/source/content/xbl/builtin/win/platformHTMLBindings.xml#35

Sounds okay. Sorry for the delay.

Patch bitrotted due to the Makefile DIRS refactor

Comment on attachment 720661 [details] [diff] [review] Add Android XBL key bindings (v5) The interdiff looked reasonable on first sight to Ms2ger :-)

Try: https://tbpl.mozilla.org/?tree=Try&rev=725082226a3e https://hg.mozilla.org/integration/mozilla-inbound/rev/b224ddd1e13d https://hg.mozilla.org/integration/mozilla-inbound/rev/e019954a133c https://hg.mozilla.org/integration/mozilla-inbound/rev/c124fcfa7414

https://hg.mozilla.org/mozilla-central/rev/b224ddd1e13d https://hg.mozilla.org/mozilla-central/rev/e019954a133c https://hg.mozilla.org/mozilla-central/rev/c124fcfa7414

Test Cases added in Full Functional Tests Suite: https://moztrap.mozilla.org/manage/case/7406/ https://moztrap.mozilla.org/manage/case/7407/