207936 - [trunk] JA IME: cursor position is off before and after text is committed

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Katsuhiko Momoi]

** Observed with 2003-05-30 build **

This is exactly the same bug as Bug 204434. But for bug management
purposes, this new bug is created to pursue the solution on the trunk. 
For 1.4 branch, we agreed to turn off the offset cache as described in
Bug 204434, which will be closed once the branch checkin is completed 
and verified.

Let me summarize the issue here briefly. For those interested in full
discussions, please see the original bug.

Comment 1

[Katsuhiko Momoi]

From Bug 204434 comment #21, 

The root probelm for this bug seems to be that the cursor position
is somehow remembered for the alphabetic character and when the
wide character is inserted, the position is off by the amount of the
space between that single-byte alphabetic charcter and the Asian character.

This delayed cursor position porblem is observed under 2 conditions 
in Japanese:

1. When the text input is still not commited.
2. After the text input has been committed.

(Note: In Chinese you see this problem under condition #2 only usually.)

#2 is particularly bad because the user is supposed to be moving to the
input for the next word and should not be seeing the cursor moving back
or forward to delay this action.

You can see this problem also in Chinese input methods.
Frank Tang and I experimented with this and this is how you can reporduce
this problem:

1. Choose the keyboard Chinese (PRC).
2. Choose the input method, QuanPin or ShuanPin.
3. Open Composer and input "a" or any other single vowels
   like "i", "u", "e", etc.
4. When the Chinese character selection box comes up, choose
   any character and commit. You can see the cursor usually half character
   to the left of the real position and then move back to the right 
   position.

This affects not only Japanese but also Chinese. In the case of Japanese,
it is particualrly bad affecting the user's ability to input characters
smoothly. This to me is a blocking bug for release in Asia, and particularly
in Japan, where more than 10% of Gecko downbloads occur. Display and Input
are bread and butter functions for browser/mail/composer. This kind of
fundamental problem cannot be ignored. Setting Milestone to 1.4 Final.

Comment 2

[Katsuhiko Momoi]

Leon, I tried your patch in Bug 204434 comment #64, but it did not
fix the problem for Japanese IME. The build I tried is a trunk source
from 2003-05-30. The proposed patch is:

http://bugzilla.mozilla.org/attachment.cgi?id=124601&action=view

Comment 3

[leon.zhang]

momoi, thx for your testing.
I will try to give another patch.
can you discribe the whole caret displying process?
  1) when type 'a',  the caret pos is correct? 
  2) then commit the selected word, caret pos is correct?

btw, I am trying to check in those pathes for bug 204434.

Comment 4

[leon.zhang]

can you type a chinese word 啊， the first selected word when  typpe 'a'.
tell me the whole process. thx. (I tried that patch in my chinese window box,
looks ok.)

Comment 5

[leon.zhang]

copy some of kin's comments here:
http://bugzilla.mozilla.org/show_bug.cgi?id=204434#c45

Leon, I'm just curious, is this bug due to the fact that when you call
SetCanCacheFrameOffset(PR_FALSE), you aren't clearing out the cached frame and
pos ... which means that if it is ever turned back on again, there is a chance,
even after the reflow, that you might get a match and use a stale point?

Up to now, I have read kin'c comments in detail.
I think kin's comment is helpful.

I have tried clear out some content of cached info, and produce a patch:
http://bugzilla.mozilla.org/attachment.cgi?id=124601&action=view
. Maybe this cleaning action is not enough.

Comment 6

[leon.zhang]

Attachment: proposed fix

I test it in Chinese windows box using QuanPin, looks ok.
I am not sure in japanese env.

Comment 7

[leon.zhang]

momoi:
about comments #2:
I have test http://bugzilla.mozilla.org/attachment.cgi?id=124601&action=view
in my chinese windows box, I think it can resolve "2. After the text input has
been committed." when typing chinese words.

Comment 8

[leon.zhang]

Attachment: previous patch: clean cache when turn off caret cache

move proposed patch from bug 204434 to current bug.

Comment 9

[Katsuhiko Momoi]

Leon，I applied your patch:

http://bugzilla.mozilla.org/attachment.cgi?id=124798&action=view

to the current trunk build. It did not solve the Japanese
IME problem. Try the following.

1. Turn on JIME (MS)
2. open an editor and start typing: k o r e  (ignore the spaces)
3. At this point, see where the caret is. It is at the middle
   of a character "re" rather than at the end.
4. Now commit this character without conversion by pressing the
   Enter key. Tne observe where the caret is. It is still in the 
   middle of the charcter "re" and then after one tick, moves to the
   correct position.

Try the same steps above with the following input (ignore the spaces):

A) z u t t o   (Note that caret is one character to the left of where
                 it should be.)
B) m o (Note that the caret is now to the rihgt of the the character
        "mo". After commiting it, it moves back left.)

Comment 10

[Kathleen :Brade]

It's interesting on my macho trunk build, I see the caret blink once in the
wrong place and then it blinks in the correct location after that (current
behavior without Leon's patch).

Comment 11

[Katsuhiko Momoi]

Regarding comment #10 by brade, that is exactly what is wrong. That 
delay in correcting the caret position -- after the text is already 
committed -- prevents users from experiencing normal Japanese input 
behavior. 

How much of hindrance this "one" blink feels to the user depends 
on the combination of input characters. In some cases, the cursor 
moves 3-4 character position to the right and then back. For example,

1. Type English words in such a way that it's ready to break a line
   with one or two more characters. e.g. ....... New York
2. Now turn on Japanese IME and type: w o
3. Then commit by pressing the Enter key.

You can see that if the line breaks with the "wo" input in step 2, the Japanese
character will move to the next line and then when you commit this character,
the caret flies 2-3 characters to the right and then moves back. 

As you can see, the severity of the problem depends on what single-byte
characters are being entered to represent the Japanese characters and their
width calculation. If this problem interacts with line-breaking, it produces
a jarring effect as described above.

Comment 12

[leon.zhang]

Attachment: my test result using JIME(region is japanese)

I am not sure it is the correct result.
applied patch(http://bugzilla.mozilla.org/attachment.cgi?id=124798&action=edit)

Comment 13

[leon.zhang]

Attachment: another old patch

Comment 14

[Katsuhiko Momoi]

With regard to: http://bugzilla.mozilla.org/attachment.cgi?id=124869&action=view
the images show the half of the problem I described above.

In the 3rd image (type "r"), you see that the caret position is in 
the middle of the character rather at the right edge. If you wait one
blink, it will shift to the right. 
The other half of the problem comes when you commit the text by pressing 
the Enter Key. The caret moves left to the middle of the 2nd character for one
blink/tick and then moves back. 

Comment 15

[leon.zhang]

about 'r': if you type it in NS7.0, the caret pos is the same.
when commit, I still find the caret's pos is correct, or same to NS7.0

Comment 16

[Katsuhiko Momoi]

Withregard to comment #15, I disagree with you. Mozilla 1.0.2 branch or
Netscape 7.02 does not have this problem. The current problem is
not acceptable for Japanese IME.

Comment 17

[leon.zhang]

Attachment: another image(region is chinese)

my windows 2000 box is chinsese version.

In "control panel", if I my region is "japanese" in "general" page, after
applied patches above, we can get pictures
http://bugzilla.mozilla.org/attachment.cgi?id=124869&action=view

If I set region is "chinese" in "general" page, after patches above, all is
correct. e.g. current picture.

so I think this bug main focus	should be:
caret blink once when type a chinese/japanese word.
Patche(http://bugzilla.mozilla.org/attachment.cgi?id=124801&action=view)
seeking r/sr can reslove it.

Comment 18

[leon.zhang]

so I think main focus of this bug should be:
caret blink once in the middle of a commied chinese/japanese word, then jump to
the correct pos.

Comment 19

[Katsuhiko Momoi]

leon: i tried your patch (remade content/base/src and layout dirctories.)
It seems to work Ok for Japanese and Chinese. I would like 
Japanese developers on this bug to try out the patch:

http://bugzilla.mozilla.org/attachment.cgi?id=124801&action=view

on Mac and Linux. I tried it on Win XP so far.

Comment 20

[leon.zhang]

Comment on attachment 124801 [details] [diff] [review]
previous patch: clean cache when turn off caret cache

kin, sfraser:
pls review this patch, I think it can improve current cache for caret, and
resolve this bug at same time.

Comment 21

[kinmoz]

Comment on attachment 124801 [details] [diff] [review]
previous patch: clean cache when turn off caret cache

Fix the following and you have an sr=[e-mail] ...

==== Why are you checking for a non-null |mCachedOffsetForFrame| if it's
already been used above the code you added? In fact looking at the entire
method, perhaps you should be checking for a null |mCachedOffsetForFrame| in
the block above that allocates it, and if it's null return an error. Same check
for null should be in |GetCachedFrameOffset()| too since it can also allocate
it.


@@ -5366,6 +5366,11 @@

   mCachedOffsetForFrame->mCanCacheFrameOffset = aCanCacheFrameOffset;

+  // clean up cached frame when turn off cache
+  if (!aCanCacheFrameOffset && mCachedOffsetForFrame) {
+    mCachedOffsetForFrame->mLastCaretFrame = nsnull;
+  }
+
   return NS_OK;
 }

Comment 22

[leon.zhang]

mCachedOffsetForFrame is allocted only if its value is null and we will use it
at same time.

mCachedOffsetForFrame in patches can be removed, because we can ensure its value
is not null when run here.

so codes can be: 
@@ -5366,6 +5366,11 @@

   mCachedOffsetForFrame->mCanCacheFrameOffset = aCanCacheFrameOffset;

+  // clean up cached frame when turn off cache
+  if (!aCanCacheFrameOffset) {
+    mCachedOffsetForFrame->mLastCaretFrame = nsnull;
+  }
+
   return NS_OK;
 }

Comment 23

[leon.zhang]

Attachment: patch checked in

transfer r/sr to

Comment 24

[leon.zhang]

checked in

Comment 25

[leon.zhang]

Comment on attachment 124801 [details] [diff] [review]
previous patch: clean cache when turn off caret cache

I want to replace previous workaround fix in 204434 with current fix in branch
1.4 for improvment of perf.
your opinion?

Comment 26

[Asa Dotzler [:asa]]

Comment on attachment 125374 [details] [diff] [review]
patch checked in

moving approval request forward.

Comment 27

[Mike Kaply [:mkaply]]

Comment on attachment 125374 [details] [diff] [review]
patch checked in

a=mkaply

Comment 28

[Mike Kaply [:mkaply]]

Please add fixed1.4.1 keyword when this is checked in.

Comment 29

[leon.zhang]

Comment on attachment 125374 [details] [diff] [review]
patch checked in

hi,mkaply
I think you agree that I can replace old patch for 204434  with current patch,
so that we 1.4 can synchronize with solutions of trunk, right?

Comment 30

[leon.zhang]

Comment on attachment 125374 [details] [diff] [review]
patch checked in

checked into mozilla1.4