299516 - Switching out and in a field with unconfirmed inline input re-enables it incorrectly

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Alexey Proskuryakov]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; ru-ru) AppleWebKit/312.1 (KHTML, like Gecko) Safari/312
Build Identifier: 2005070108 (v0.9a1+)

Inline input area stays underlined, but isn't actually reenabled

Reproducible: Always

Steps to Reproduce:
1. Go to http://www.google.ru/advanced_search
2. Using Kotoeri (or any other input method that uses inline input), type something in the first text field 
("with all of the words").
3. Without confirming the inline input area, click in the second text field; then, back in the first
  The text is still underlined
4. Continue typing
Actual Results:  
Underlining disappears, a new inline input area is created.

Expected Results:  
Underlining should match the current inline input area.

Instead of step 4, one can try using some functions from Kotoeri menu - they do not work, as well.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Moving the IME bugs to 1.1 :-(

Comment 2

[Hiro]

The problem of looking like bug163572 occurred when the reproduction step was repeated. 
Therefore, Japanese input becomes difficult. 

It is not only by the problem of externals like underline. 

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Moving IME-related bugs to Camino 1.2 :(

Comment 4

[Hiro]

Firefox reproduces because it moved to Cocoa Widget. 
It is not a problem of Camino alone. 

Mac OS X 10.3.9
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20061215 Minefield/3.0a1

Comment 5

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

I'm working on this bug, but there may be two or more bugs...

Comment 6

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

Attachment: Patch rv0.1 (work in progress)

Comment 7

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

oops, sorry for the spam :-(

Comment 8

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

Attachment: Patch rv1.0

I think that this patch should/can fix this bug.

We have two issues for fixing this bug:
1. NSInputManager cannot commit the current transaction if the composing widget has lost focus.

We can use TSM APIs, therefore, at starting the composing, we can get the active TSM document ID and we can use it for commit the composing.

2. But FixTSMDocument which can commit the composing doesn't send insertText if the document has been deactivated, therefore, nsTSMManaer::CommitIME and nsTSMManager::CancelIME check whether the composing is finished by FixTSMDocument. If it is not finished (i.e., FixTSMDocument doesn't call insertText), we should call insertText ourselves.

Comment 9

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

Note that we should not call nsTSMManager::CommitIME from resignFirstResponder and viewsWindowDidResignKey. Because it is called via ResetInputState from the events which is risen from them. But if remove them, this patch doesn't work fine, I think this bug should be fixed, but now, I may not have much time, therefore, I hope that the patch will be landed to trunk with the bug :-(

Comment 10

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

Attachment: Patch rv1.1

O.K. This patch works fine for me.

In logically, resignFirstResponder and viewsWindowDidResignKey should not call nsTSMManager::CommitIME(). Because it should be called nsIMEStateManager via ResetInputState(). However, it doesn't work fine only on Mac... (It should be separated to another bug now.)

Comment 11

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

Comment on attachment 274334 [details] [diff] [review]
Patch rv1.1

oops, sorry for the spam.

the patch has a bug.

Comment 12

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

Attachment: Patch rv1.2

fix two memory leak bugs and using NSString for sComposingString, this is simpler.

Comment 13

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

roc:

would you sr this?

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 274335 [details] [diff] [review]
Patch rv1.2

+      bufPtr[0] = (PRUnichar)'\0';

Constructor cast please

Comment 15

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

Comment on attachment 274335 [details] [diff] [review]
Patch rv1.2

Thank you, roc.

I need the approval for this bug. This bug blocks next my work, I want to land this quickly.

Comment 16

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

Attachment: Patch rv1.3 (for checking-in)

Comment 17

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

checked-in.