645974 - Switching focus with uncommitted IME text does not commit or send events, and leaves corrupted text state.

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Elliot Block]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0

If there is uncommitted IME text, when switching focus the browser should either auto-commit the text or at least send an event so that Javascript can force-commit the text.  Today the browser provides neither, and after the user has switched focus weird unstable text gets left in both the elements you switched from, and the element you switched to.

Reproducible: Always

Steps to Reproduce:
1. Using Japanese IME, type some text in a designMode element and do not commit.
2. Using the mouse, switch focus to another element.

Actual Results:  
The text you were typing is not committed.  

No event sent to indicate the typing session lost focus.

The text you were typing gets duplicated into the second element.

The text you were typing is visually still in the first element but when you put focus back in it, the text was not really there.

Expected Results:  
The text you were typing is committed into the first element. 

An event is sent to indicate the typing session lost focus.

The text you were typing does not get duplicated into the second element.

Comment 1

[Elliot Block]

Attachment: Plain demonstration of uncommitted IME focus switching bug.

Comment 2

[Elliot Block]

Also note this appears to be present in both FF 3.6 and 4.0.

Comment 3

[Makoto Kato [:m_kato]]

confirmed latest nightly of Win32 and MacOSX

Comment 4

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

This should be a DOM event handling issue rather than editor.

If there is a composition, all composition events on same top level window must be dispatched to composing element. But I've had no idea yet, what class should manage the last composition element in every *top level* window? nsIMEStateManager is static, so, it cannot do it. nsFocusManager is a singleton, it's not either.

Comment 5

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

Hmm, the approach in my previous comment isn't enough. When the tab is being detached from its window, the composition should be committed (or discarded) *before* the detaching. If we don't do so, wrong transaction might remain on the old window.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Why would we need to keep reference to different composition elements?
I mean one in each top level window?

Couldn't there just be 2 global variables. previousCompositionElement and currentCompositionElement
When focus changes, composition could have already moved to currentCompositionElement, but we'd need to undo/commit the possibly 
uncommitted previousCompositionElement.

Comment 8

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

Oops, sorry, I haven't realized your comment.

(In reply to Olli Pettay [:smaug] from comment #6)
> Why would we need to keep reference to different composition elements?
> I mean one in each top level window?

We can assume that one composition is available on one native IME context. With IMM on Windows, our process has only one native context. On Linux and Mac, our process has one context per top level window. I think that we need to manage composition transaction per native IME context.

> Couldn't there just be 2 global variables. previousCompositionElement and
> currentCompositionElement
> When focus changes, composition could have already moved to
> currentCompositionElement, but we'd need to undo/commit the possibly 
> uncommitted previousCompositionElement.

Currently, our editor tries to commit composition string forcibly when it loses focus or some methods which will change selection or content are called. The cause of this bug is, the focus change causes different presShell handles the composition events of the committing.

My patch for bug 705057 fixes this bug, please check it.

Comment 9

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

FYI: I'm planning to remove the forcibly commit (I think that it's bad custom of our editor) as far as possible in the future.

Comment 10

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

Fixed by bug 705057. It contains automated tests for this bug.