Closed Bug 194559 Opened 22 years ago Closed 20 years ago

Entered Japanese text doesn't go back to preedit text by Ctrl+BS

Categories

(Core :: Internationalization, defect)

Product:
Core
Component:
Internationalization
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: kazhik, Assigned: smontagu)

References

Details

(Keywords: inputmethod)

Attachments

(1 file, 5 obsolete files)

patch rv.5(comment rewrote)
2.89 KB, patch
emaijala+moz
: review+
roc
: superreview+
Details | Diff | Splinter Review 
Entered Japanese text doesn't go back to preedit text by Ctrl+BS.

Steps to reproduce:
(1) Type <kanji> with IME and hit and Enter key.
(2) Press Ctrl+BS.

Expected result: <kanji> goes back to preedit text.

Actual result: <kanji> is displayed as an another preedit text.

Original report in Bugzilla-jp:
http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=2885
This bug is reproduced with ATOK14,ATOK15 and WXG 4.03.
Mager Japanese IME ATOK recent version and some other IME throws messages
direffent order from MS-IME.
When we push reconvert key, MS-IME send
1. WM_KEYDOWN (bs)
2. WM_CHAR (bs)
3. WM_KEYUP (bs)
4. 1-3  several times (the number of last converted strings).
5. WM_IME_STARTCOMPOSITION preedit string.
but ATOK for win send
1. WM_KEYDOWN (bs) several times (the number of last converted strings).
(not delete last concerted strings yet)
2. WM_IME_STARTCOMPOSITION preedit string.
3. WM_CHAR (bs) several times (the number of last converted strings).
(cause EndComposition and we delete strings of '2.' not last converted strings!)
4. WM_KEYUP (bs) once.
5. next WM_IME_COMPOSITION ...
Attached patch Patch 1 (obsolete) — — Splinter Review 
This patch is following functions.

1. Backup the information of IME Composing.
2. Cancel the Current IME session.
3. Create the New IME session that is makes from backup data.

Now, if it is composing when OnChar Event is run,
the IME session is end.
but, then <Ctrl+BS>, the IME session should be none.
because, the VK_BACKSPACE is need for the text that must be deleted.
And the text should come back after KEY_PRESS event.
Comment on attachment 144109 [details] [diff] [review]
Patch 1

crashed on win9x
Attachment #144109 - Attachment is obsolete: true
Attachment #144109 - Flags: review?(ere)
I'm trying to get the patch in bug 197474 in as soon as possible when the trunk
is opened for 1.8a. It seems both bugs touch some same code. Would it be
possible to do the patch here against the nsWindow version patched with the
changes from 197474?
Ere:

This patch don't works fine on Win98(Mozilla crashed),
and some minor problems exists on WinNT.
The cause of a problem isn't known, and I don't have win9x system.
So, this patch's problem cannot immediately be solved.

If all problem is solved, I will demand review again.
At that time, It will rewrite, if there is necessity.
# This bug of may be unable to be fixed...
I will attach the new patch soon.
But new patch having minor problem that is on ATOK only.
The problem is impossible to fix.
When using ImmSetCompositionString with parameter as GCS_COMPSTR, ATOK crash
or doesn't operate well.
That is bug of ATOK.
Because, On WXG, first patch works fine.

For fixing this bug completely, we need to rewrite large code in nsEditor.
But it is not realistic.
So, I tryed to create the patch that is "ad hoc".
The basic concept of new patch is same as comment 3.
But I separate the code as following.
1. The code is used by ATOK.
2. Other code is used by Other IME(However, all IME that this bug doesn't 
   reproduce, that isn't included: e.x., MS-IME).

It isn't good patch, but I don't have better idea.
However, this bug should be fixed.
Because ATOK and WXG is major IME in Japanese people.
Comment on attachment 144626 [details] [diff] [review]
patch v.2

Please review this patch before rewrite.

# I will rewrite the patch after bug 197474 checking in.
Attachment #144626 - Flags: review?(ere)
Comment on attachment 144626 [details] [diff] [review]
patch v.2

I'm not sure if I'm good in reviewing IME stuff, but I can try. With a quick
glance, there's one question: could we do without LockWindowUpdate? It can
cause horrible window flashing in some circumstances. The locked window behaves
well, but all the others flash. This is most evident when Mozilla is not
running full-screen.
When LockWindowUpdate was removed, the text will flash.
LockWindowUpdate is necessary.

SDK said,
"If the function fails, the return value is zero, indicating that an error
occurred or another window was already locked. "

If we need another way, we need to lock window mechanism when WM_PAINT.
Comment on attachment 144626 [details] [diff] [review]
patch v.2

Ok, I don't know the IME so I just have to take your word for it. Technically,
there are just a couple of things:

I really don't like LockWindowUpdate. I wouldn't be seeing its effect, but I
don't want it there. If the flicker is too bad, suppress WM_PAINTs and repaint
when you're done. 

> +        if (!memicmp(imeFileName, "ATOK", 4))

I think memicmp is MSVC++ specific. If so, MinGW doesn't support it and this
would break compiling with it.

One nit:

> +  if (mIMEIsComposing && virtualKeyCode == 0x8) { // BackSpase

Should be BackSpace.
Attachment #144626 - Flags: review?(ere) → review-
>> +  if (mIMEIsComposing && virtualKeyCode == 0x8) { // BackSpase
> 
> Should be BackSpace.

Why?
If mIMEIsComposing is removed, we will have performance issue.
Because, unnecessary API will run.
to Masayuki Nakano
Ere seems to say your 'Typo' problem.
you must check 'nit' with dictionary.
Attached patch patch rv.3(Test Ver.) (obsolete) — — Splinter Review 
Ere:

This patch is not using LockUpdateWindow API.
But, When the text is edited, nsEditor don't use WM_PAINT for repainting the
content.
# printf("Skip Paint.") is not run.
So, I can't stop flicker in WM_PAINT...

Do you have good idea?
Attachment #144626 - Attachment is obsolete: true
Attached patch patch rv.4 (obsolete) — — Splinter Review 
This patch do _not_ stop flicker.
However, in many case, the flicker is one time.

We try to test this patch.
Attachment #145449 - Attachment is obsolete: true
(In reply to comment #12)
>(From update of attachment 144626 [details] [diff] [review])
>> +  if (mIMEIsComposing && virtualKeyCode == 0x8) { // BackSpase
>Should be BackSpace.
If you used VK_BACK in the first place the comment would be unnecessary :-P
Attachment #145456 - Attachment is obsolete: true
Attached patch patch rv.5 (obsolete) — — Splinter Review 
This is new patch for new architecture(by bug 197474).
The attachment 142468 [details] [diff] [review] cannot fixed this problem.
Because the patch is premised on the message que that having
the pair of WM_KEYDOWN and WM_CHAR or WM_KEYDOWN and WM_SYSCHAR,
but ATOK and WXG's "Kakutei-Undo" is not create the message pair.
In these cases, the message que is following:
 WM_KEYDOWN(VK_BACK) * n
 WM_KEYUP(VK_BACK) * 1 (ATOK only)
 WM_IME_STARTCOMPOSITION(wParam = 0x0, lParam = 0x0)
 WM_IME_COMPOSITION(wParam = 0x0, lParam = 0x1BF)
 WM_CHAR(VK_BACK) * n
 WM_KEYUP(VK_BACK) * 1
In the line of 2992th GetMessage API cannot run, because the message is not
WM_CHAR and WM_SYSCHAR.
The message may be WM_KEYDOWN or WM_KEYUP by ATOK or WXG.

We are testing this patch by Japan testers.
The test build is:
http://www.d-toybox.com/mozilla/testbuilds/bug2885.zip
Adding to Cc.

Momoi-san:
This problem should be tested by other IME user; i.e., Chineses people, and
Korean people.
Do you know that who can test on these system?

I hope that this bug is fixed in 1.8a2.
Because the patch should be tested by many and various IME users.
Attachment #150255 - Flags: review? → review?(ere)
Comment on attachment 150255 [details] [diff] [review]
patch rv.5(comment rewrote)

Technically it looks good to me. I trust you know it works correctly. r=ere
Attachment #150255 - Flags: review?(ere) → review+
Please super-review the patch.

This is major bug and the patch should be tested by many IME users.
So this patch should be checked in before 1.8a2.
Flags: blocking1.8a2?
Comment on attachment 150255 [details] [diff] [review]
patch rv.5(comment rewrote)

I chage super-reviewer to same as Bug 244334.
Attachment #150255 - Flags: superreview?(bryner) → superreview?(roc)
Attachment #150255 - Flags: superreview?(roc) → superreview+
Please check in before 1.8a2.
Flags: blocking1.8a2? → blocking1.8a2-
Fix checked in for Masayuki.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: