721393 - Virtual keyboard enter key doesn't work correctly in designMode document

Status: RESOLVED FIXED

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

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, steps to reproduce:
- Type 'a' with the vkb
- Type Enter with the vkb
- Type 'a' with the vkb
- Type Enter with the vkb
- Type 'a' with the vkb
- Type Enter with the vkb

Expected result:
a
a
a

Actual result:
aaa

While typing in the designMode document, I get often encounter bug 720125, btw.

This was tested on the LG Optimus Black, Android 2.2.2 in portrait mode.

Comment 1

[Martijn Wargers (dead)]

Hmm, I can only reproduce with the dom.w3c_touch_events.enabled pref set to true in about:config.
So looks like a regression from bug 603008.

Comment 2

[Alex Pakhotin (:alexp)]

(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #1)
> Hmm, I can only reproduce with the dom.w3c_touch_events.enabled pref set to
> true in about:config.

Doesn't seem to be depending on that - I can reproduce it with that pref set to default value of false.

Different IMEs have different behavior. Most, including HKB, move the cursor to a new line, but the next typed character appears on the previous line, with the cursor after it. SwiftKey X adds a space instead of going to a new line.

Martijn, can you please try to find a regression window? I'll also check the key dates, when I had major changes to IME. Unfortunately, focusing in the iframes with designMode=on was broken until recently, it might be difficult to test typing in the build where that didn't work.

Comment 3

[Alex Pakhotin (:alexp)]

OK, apparently this is another side-effect of my input handler refactoring made in bug 595008, which simplified the code and fixed many problems, but introduced some new ones.
The first build with this issue is 20111217031145.

Comment 4

[Martijn Wargers (dead)]

Ah, sorry, sometimes it seems to work (not sure when), I guess it accidentally worked when I flipped the touch events pref.

Comment 5

[Alex Pakhotin (:alexp)]

One more observation.
It looks like specific to the designMode=on. Input seems to work fine in a multi-line editor implemented with <div contenteditable="true">. I am trying it on my test page at http://people.mozilla.org/~alexp/test/edit.htm

Comment 6

[Alex Pakhotin (:alexp)]

Another finding. It seems like not only because of my refactoring.
The old implementation of GeckoInputConnection has the same problems with Enter key in a document with designMode=on when using SwiftKey X or hardware keyboard.

Comment 7

[Alex Pakhotin (:alexp)]

bug 595008 did affect some aspects of this - the standard Android and Swype keyboard stopped working after that, but hardware keyboard and SwiftKey X had the same issue in the builds before that change, for example I tried 20111120040250, which has the problem. So, would be nice to find a real regression range.

Comment 8

[Martijn Wargers (dead)]

I can reproduce this bug also in Fennec 9, this is XUL Fennec, so this is not a regression from the move to Native Fennec.
I have my doubts there is a regression here. I tried older versions, but there I get worse bugs, making it impossible to see this bug.
In Fennec 4, typing doesn't work at all with the testcase.
In Fennec 6, the vkb doesn't come up when focusing the designMode document.

Comment 9

[Alex Pakhotin (:alexp)]

(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #8)
> I can reproduce this bug also in Fennec 9, this is XUL Fennec, so this is
> not a regression from the move to Native Fennec.
> I have my doubts there is a regression here. I tried older versions, but
> there I get worse bugs, making it impossible to see this bug.

Thank you Martijn. At least this excludes the recent changes related to native UI. But apparently now it requires more deeper investigation of the Midas editor internals.

Comment 10

[Alex Pakhotin (:alexp)]

Got some answers to my questions from Masayuki:

> Is there any specifics to enter multi-line text with IME? Is
> NS_TEXT_TEXT with just a new line character in it supposed to be handled
> properly? Doesn't it require any special processing?

Hmm, I'm not sure. Normally, CJK desktop's IMEs don't input line-breakers via composition. Enter key native events are usually used for committing the composing string (processed by IME) or passed to the application (IME ignores it if not composing).

So, I think that it's not tested NS_TEXT_TEXT event handling which include line-breakers. You can test it by using nsIDOMWindowUtils. See this automated tests:
http://mxr.mozilla.org/mozilla-central/source/widget/tests/window_composition_text_querycontent.xul

synthesizeComposition() and synthesizeText() dispatch NS_COMPOSITION_EVENT or NS_TEXT event. You can emulate any cases by using them.

If some Android's VKBs include line-breakers in composition string, we should dispatch events same as WebKit for Android for the compatibility. I.e., if a composition string is separated by line-breakers and Enter key events are synthesized, we should emulate them. Otherwise, composition string can include line-breakers, you should dispatch it normally. You can check the events by following testcase:
https://bugzilla.mozilla.org/attachment.cgi?id=588374

If our editor doesn't accept line-breakers via composition and it blocks bug 721393, we should fix it first.

> When typing text in a document/IFRAME with designMode=on there are no
> nsWindow::OnIMETextChange() calls, unlike when editing a regular INPUT
> box, or a DIV with contentEditable=true. Do you know why is that?

Oh, I didn't realize it. It should be called on designMode editor too. Could you file a bug for it and CC me? I guess that it's failed to listen to DOM mutation events. Document nodes are responsible for handling all events if in designMode. This is the most different point from other editors (Element nodes are responsible for all events for other editors).

Unfortunately, I don't have much time this month for the investigating for you since I have a lot of large and important bugs. But I can advise you. Feel free for asking me. I'm working on getting rid of NS_TEXT_TEXT events, so, I need to know every change for composition in widget layer.

Comment 11

[Alex Pakhotin (:alexp)]

Attachment: Fix

This fix seems to work. I'd like to test it a bit more though.

Comment 12

[Alex Pakhotin (:alexp)]

Attachment: Patch 2

The patch fixes Enter, Delete and arrow keys in an IFRAME with designMode=on, but there are still some weird things happening on Etherpad.

Comment 13

[Alex Pakhotin (:alexp)]

Comment on attachment 595602 [details] [diff] [review]
Patch 2

This patch is definitely an improvement, it fixes the current issue and improves bug 719121. I think we should go with it and do more testing.
The weirdness on Etherpad seems to be due to the bug 725919.

Comment 14

[Alex Pakhotin (:alexp)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f9599a2d85a5

Comment 15

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/f9599a2d85a5