59405 - IME candidate window position is not correct in text field on New card and To field

Status: VERIFIED FIXED

(MailNews Core :: Internationalization, defect, P3)

Description

[Teruko Kobayashi]

This is separate bug from 53072.
IME candidate window position is not correct in text field on New card of 
Address book and To: filed of Mail Composer.

Steps of reproduce 
1. Open Message composer
2. Turn on IME
3. Move the cursor to "To:" field
4. Type hiragana character, like "にほん"
5. Hit space twice to open Candidate window

Look at the position of the Candidate Window. It opens below the subject field.

This happens in each text field on New card of Address book, also.
This does not happen in each text field on New list of Address book.

From Katakai-san's comment in the 53072.
"On Linux, the pre-edit position
is not correct when the input style is over-the-spot."

Tested 11-06 MN build.

Comment 1

[Shanjian Li]

I believe this is a very serious problem. I will keep it on my 
radar for a little while. Katakai, please help on this bug. I 
might also spend sometime on this one. I will post my progress 
to the bug report. 

I suspect the problem exist somewhere in nsCaret.cpp, function
nsCaret::GetViewForRendering. Especially line around 610 is 
very suspectious.

 

Comment 2

[Shanjian Li]

Attachment: proposed fix

Comment 3

[Shanjian Li]

I tried the above patch, it seems fix the problem. Simmon, can you take a 
look at the fix?

Comment 4

[Simon Fraser [no longer active]]

It seems that this patch will cause the method to return coordinates relative to 
the enclosing widget for the container of the caret, but not always relative to 
the top-level window. Walking to the top of the view hierarchy seems to be the 
right thing to do (as the code does now).

Perhaps this code loop needs to do the same scrollableView stuff as the loop 
above?

Comment 5

[Shanjian Li]

IME code need the position relative to widget that containing the caret. Is 
there any case that the reply position need to be relative to top level window?
Besides IME, is there other code that need this position? If so, we might need 
to define a new message which returns position that IME needs.

Comment 6

[Simon Fraser [no longer active]]

The caret code was designed to return either [top-level]window-relative 
coordinates, or parent-view relative coordinates. If you need an offset to the 
parent widget, then you'll need a new method on nsICaret 
(GetWidgetRelativeCoordinates), a new value in the EViewCoordinates enum, and 
then some code that does what your patch does to return those.

Comment 7

[Shanjian Li]

Understood. But do you know why it is designed to return top-level relative 
position, ie., who need top-level relative position? Just want to make sure 
that the design can not be changed.

Comment 8

[Simon Fraser [no longer active]]

I thought that IME wanted window-relative coordinates, not widget-relative. 
Regardless, I think the window-relative code should stay; it might be used by 
drag-and-drop, or accessibility code, or something.

Comment 9

[Shanjian Li]

Before I writing new codes, I need to clarify several issues. 
I do not quite understand how widget is define relative to window. In 
addressbook->new card, I assume each edit box correspond to one widget.
Simon, Is that right? On windows platform, each widget does not has a 
correspondent native window. In above case, all those field are contained
in one native window. 
I am not sure how top-level window is defined. In IME, we need the position
relative to the lowest level native window which contain the widget. In 
composer, the lowest level native window happens the same as top-level window.
  

Comment 10

[Simon Fraser [no longer active]]

Edit fields do not have a native window, that is correct.

When I talked about "window-relative" coordinates here, I was *always* talking 
about the top-level window (i.e. I was using window in the Macintosh sense, not 
the Windows "widget" sense).

So what you need is nearest-native-window coordinates. How does this translate to 
platforms that don't have native, non-top-level windows (like Mac)?

Comment 11

[Teruko Kobayashi]

Shanjian, I tested this in 2000-11-06 MN Mac build.  The candicate
window is displayed at just above on "To:" field and each text field
of New card.  I think this is same as simple text editor of Mac.

Comment 12

[Shanjian Li]

Naoki, can you try my patch on mac? I am not intending to use that patch.
But the behavior on mac may give me some idea about how to implement a new
patch which will work on all 3 platform. 

Comment 13

[Shanjian Li]

Attachment: mac screen shot before my 1st patch

Comment 14

[Shanjian Li]

Attachment: mac screen shot after my 1st patch

Comment 15

[Shanjian Li]

Naoki helped test my patch on mac. It break IME on mac. Simon, can you 
help find a solution that will work on all 3 platforms? I do not have 
mac env, and do not know how mac works.

Comment 16

[Shanjian Li]

nsCaret::GetWindowRelativeCoordinates got called 3 places in mozilla code, 2 
places are for IME purpose, one for handling up and down key in composer. In 3rd
place, only x position is desired. Among all those 3 places, I believe top-level
window relative position make no sense on windows and unice platform. The 3rd 
place does not cause any problem because the top-level and lowest-level window 
start from same x position in all occurences. I believe the behavior of 
nsCaret::GetWindowRelativeCoordinates should be changed, but do not know how 
to handle mac platform.

Comment 17

[nhottanscp]

Adding yokoyama to cc.

Comment 18

[Shanjian Li]

Attachment: new proposed fix that works on all platform, but with ifdef

Comment 19

[Shanjian Li]

The problem is even serious on HPUX. Composing string is unseeable in address field in new mail. 
This bug must be fixed before we could ship netscape6 for HPUX.

Comment 20

[Simon Fraser [no longer active]]

Why is HPUX different here? Is its IME implementation different?

Comment 21

[Shanjian Li]

On HPUX, only over-the-spot IME is available. That means every japanese user will experience
this problem. (On-the-spot is the default setting for linux, which only have problem with
candidate window.) On HPUX, in address field of new mail, the composing string is invisible.
That make it even worse. 

Comment 22

[Shanjian Li]

Fix has been checked in to OEM branch. Keep this bug open 
because it is not available in trunk yet.

Comment 23

[Katsuhiko Momoi]

QA contact to ji.

Comment 24

[Shanjian Li]

Attachment: New proposed patch.

Comment 25

[Shanjian Li]

Simon, from previous discussion, I hoped that I convinced you about 
the type of coordinate needed for windows and unix. They all need
"eRenderingViewCoordinates" instead of "eTopLevelWindowCoordinates".
If so, to make the choice in nsPlaintextEditor.cpp is more reasonable.
Could you take a look and give a code review?

Comment 26

[Simon Fraser [no longer active]]

sr=sfraser

Comment 27

[kinmoz]

Several of us in the editor group would like to avoid using platform specific 
ifdefs in the editor code, but both sfraser and I are willing to let this fix 
land if you added a comment that says something like:

// XXX: The following ifdef should really be pushed up into the
//      IME front-end code, or down into the caret code. There was a
//      suggestion that perhaps nsCaret::GetCaretCoordinates() should
//      be modified so that we can pass in something like eIMECoordinates
//      and let it decide what to return based on the platform we are
//      running on.


r=[e-mail]

Comment 28

[Shanjian Li]

fix checked in.

Comment 29

[ji]

Verified with 04/17 trunk build.

Comment 30

[Masaki Katakai]

Have anyone verified this fix on HPUX? or over-the-spot
on Linux/Solaris?

I tried today's nightly but it seems that the problem still
happens on Address Book.

Comment 31

[Masaki Katakai]

Attachment: snapshot on Solaris over-the-spot, cursor in First:, preedit position is not correct

Comment 32

[ji]

Reopened the bug based on Katakai-san's test results.

Comment 33

[ji]

This problem also happens on linux with input style set to over-the-spot.

Comment 34

[Shanjian Li]

I think I found the problem. My previous fix is not complete. There are 2 other places needs to be
change. I decide to adopt kin's advice and prepare a new fix.

Comment 35

[Shanjian Li]

Attachment: Newer and complete patch, part 1

Comment 36

[Shanjian Li]

Attachment: Newer and complete patch, part 2

Comment 37

[Shanjian Li]

kin and Simon,

My previous fix is not complete for unix. Could you take a look a my new fix? 
The idea is the same, but it is revised per kin's advice. thx.

Comment 38

[Simon Fraser [no longer active]]

r=sfraser

Comment 39

[kinmoz]

sr=[e-mail]

Comment 40

[Christopher Blizzard (:blizzard)]

a=blizzard for mozilla 0.9

Comment 41

[Shanjian Li]

new fix has been checked in per approval from blizzard.

Comment 42

[ji]

Checked the linux build with xim input style set to over-the-spot.
Now the candidate window is placed right below the text field. 
But the IME status window disappears after you select from the candidate window.
To see it:
1. Open new abook card window.
2. In any text field, like Firstname field, turn on IME.
3. Enter a Ja character, and press space bar to bring up the candidate window.
4. Hit Enter to commit.
The IME status window is gone, and the character is not actually commited, you
need to hit Enter again to make the character actually entered into the editing
area. And this problem is not limited to Address Book. 
I'll mark this verified and open another bug for this.

Comment 43

[ji]

The problem described above exists on the builds before shanjian's fix.
Katakai-san, is it a known problem?