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

VERIFIED FIXED

Status

MailNews Core
Internationalization
P3
normal
VERIFIED FIXED
18 years ago
10 years ago

People

(Reporter: Teruko Kobayashi, Assigned: Shanjian Li)

Tracking

({intl})

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments)

(Reporter)

Description

18 years ago
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.
(Assignee)

Comment 1

18 years ago
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.

 
Status: NEW → ASSIGNED
(Assignee)

Updated

18 years ago
Blocks: 52139
(Assignee)

Comment 2

18 years ago
Created attachment 18898 [details] [diff] [review]
proposed fix
(Assignee)

Comment 3

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

Comment 4

18 years ago
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?
(Assignee)

Comment 5

18 years ago
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

18 years ago
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.
(Assignee)

Comment 7

18 years ago
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

18 years ago
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.
(Assignee)

Comment 9

18 years ago
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

18 years ago
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)?
(Reporter)

Comment 11

18 years ago
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.
(Assignee)

Comment 12

18 years ago
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. 
(Assignee)

Comment 13

18 years ago
Created attachment 19026 [details]
mac screen shot before my 1st patch
(Assignee)

Comment 14

18 years ago
Created attachment 19027 [details]
mac screen shot after my 1st patch
(Assignee)

Comment 15

18 years ago
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.
(Assignee)

Comment 16

18 years ago
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

18 years ago
Adding yokoyama to cc.
(Assignee)

Comment 18

18 years ago
Created attachment 19325 [details] [diff] [review]
new proposed fix that works on all platform, but with ifdef
(Assignee)

Comment 19

18 years ago
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

18 years ago
Why is HPUX different here? Is its IME implementation different?
(Assignee)

Comment 21

18 years ago
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. 

Updated

18 years ago
Blocks: 60740
(Assignee)

Comment 22

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

Updated

18 years ago
Keywords: intl

Comment 23

18 years ago
QA contact to ji.
QA Contact: momoi → ji
(Assignee)

Comment 24

18 years ago
Created attachment 29842 [details] [diff] [review]
New proposed patch.
(Assignee)

Comment 25

18 years ago
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

18 years ago
sr=sfraser

Comment 27

18 years ago
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=kin@netscape.com
(Assignee)

Comment 28

17 years ago
fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 29

17 years ago
Verified with 04/17 trunk build.
Status: RESOLVED → VERIFIED

Comment 30

17 years ago
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

17 years ago
Created attachment 31409 [details]
snapshot on Solaris over-the-spot, cursor in First:, preedit position is not correct

Comment 32

17 years ago
Reopened the bug based on Katakai-san's test results.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---

Comment 33

17 years ago
This problem also happens on linux with input style set to over-the-spot.
(Assignee)

Comment 34

17 years ago
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.
(Assignee)

Updated

17 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 35

17 years ago
Created attachment 31472 [details] [diff] [review]
Newer and complete patch, part 1
(Assignee)

Comment 36

17 years ago
Created attachment 31473 [details] [diff] [review]
Newer and complete patch, part 2
(Assignee)

Comment 37

17 years ago
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

17 years ago
r=sfraser
a=blizzard for mozilla 0.9
(Assignee)

Comment 41

17 years ago
new fix has been checked in per approval from blizzard.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED

Comment 42

17 years ago
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.
Status: RESOLVED → VERIFIED

Comment 43

17 years ago
The problem described above exists on the builds before shanjian's fix.
Katakai-san, is it a known problem?
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.