717147 - input events during composition are not trusted events

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch part.1 input events which are fired after compositionupdate should be trusted events

The text event handler of editor doesn't make nsAutoEditorKeypressOperation instance. Therefore, the input events during compsition (and at commit the composition) are not trusted events.

With the attached patch, input events during composition become trusted events. However, it causes some regressions in chrome UI.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

input event handlers:

http://mxr.mozilla.org/mozilla-central/search?string=addEventListener%28\%22input\%22&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
http://mxr.mozilla.org/mozilla-central/search?string=event%3D\%22input\%22&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
http://mxr.mozilla.org/mozilla-central/search?string=oninput%3D&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch part.2 Don't do autocomplete during composition

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch part.3 Findbar shouldn't do incremental search during composition

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch part.2 Don't do autocomplete during composition

autocomplete shouldn't show popup during composition because it disturbs IME's candidate window. See bug 165055's screenshots.

The new test is copied from editor/libeditor/text/tests/test_bug527935.html, therefore it's in the path rather than under toolkit/.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch part.3 Findbar shouldn't do incremental search during composition

Findbar shouldn't find a word for unconverted composition string.

For example, if user wants to convert "ABC" to "DEF" by IME, user types "ABC" by keyboard. Then, if findbar did incremental search in "DEFABCDEF", the "ABC" would be selected. And when the user did convert to "DEF", findbar found the *second* "DEF". It's bad behavior. The first "DEF" should be selected.

Therefore, I think that findbar shouldn't do incremental search during composition.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 587686 [details] [diff] [review]
Patch part.3 Findbar shouldn't do incremental search during composition

nit: for tests, the comment should be an assertion about the correct behavior, not a description of the error. So you should have:

>++      ok(gFindBar.hidden, "Failed to close findbar after testNormalFindWithComposition");

-> "findbar should be hidden after testNormalFindWithComposition"

>+         "testNormalFindWithComposition: find field is not focused");

-> "testNormalFindWithComposition: find field should be focused"

>+         "testNormalFindWithComposition: failed to find at committing composition '" + searchStr + "'");

-> "testNormalFindWithComposition: text should be found after committing composition"

etc. This applies to a bunch of places in the existing test, it looks like...

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> Jean-Yves Perrier [:teoli] 2012-01-11 08:02:04 PST
> Keywords: dev-doc-needed

Why?

Comment 8

[Jean-Yves Perrier [:teoli]]

:masayuki: I wasn't sure, so I put the dev-doc-needed so that we could track if some doc is needed.

So you do think this is only an "implementation fix" change that doesn't need to be documented?

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Jean-Yves Perrier [:teoli] from comment #8)
> So you do think this is only an "implementation fix" change that doesn't
> need to be documented?

Yes.

Comment 10

[Justin Dolske [:Dolske]]

Comment on attachment 587684 [details] [diff] [review]
Patch part.2 Don't do autocomplete during composition

Review of attachment 587684 [details] [diff] [review]:
-----------------------------------------------------------------

Looks reasonable. Do any of the other ::Handle* methods need a similar treatment?

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Justin Dolske [:Dolske] from comment #10)
> Do any of the other ::Handle* methods need a similar
> treatment?

In the future, they might need. Currently, key events are not still fired during composition. However, D3E spec thinks only keydown/keyup events should be fired even during composition. When we fix it, we need to do it, maybe.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f64197c3878
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a71337e5cf7
https://hg.mozilla.org/integration/mozilla-inbound/rev/78e58e679fca

Comment 13

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/1f64197c3878
https://hg.mozilla.org/mozilla-central/rev/1a71337e5cf7
https://hg.mozilla.org/mozilla-central/rev/78e58e679fca