296687 - TSM (IME) candidate window is displayed at wrong position (too below)

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P1)

Description

[Katsuhiro MIHARA]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050603 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050603 Firefox/1.0+

original report written in Japanese:
http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=4450

Like Bug 235850 and Bug 9526, TSM (IME) candidate window is displayed at wrong
position (too below).

In editing TextField or TextArea, ::LocalToGlobal() revises Point too bellow.

mozilla/widget/src/mac/nsMacEventHandler.cpp, line 1904 - 1917
> nsresult nsMacEventHandler::HandleOffsetToPosition(long offset,Point* thePoint)
> {
> 	thePoint->v = mIMEPos.y;
> 	thePoint->h = mIMEPos.x;
> 	printf("local (x,y) = (%d, %d)\n", thePoint->h, thePoint->v);
> 	WindowRef wind =
reinterpret_cast<WindowRef>(mTopLevelWidget->GetNativeData(NS_NATIVE_DISPLAY));
> 	nsGraphicsUtils::SafeSetPortWindowPort(wind);
> 	Rect savePortRect;
> 	::GetWindowPortBounds(wind, &savePortRect);
> 	::LocalToGlobal(thePoint);
> 	printf("global (x,y) = (%d, %d)\n", thePoint->h, thePoint->v);
> 
> 	return PR_TRUE;
> }

A case, main window touch menu bar, but ::LocalToGlobal() revise Point.v more
than 100px.

> local (x,y) = (131, 96)
> global (x,y) = (130, 204)


Reproducible: Always

Steps to Reproduce:
1.Edit japanese in TextField or TextArea
2.
3.

Actual Results:  
TSM(IME) candidate window touchs edited line.

Expected Results:  
TSM(IME) candidate window is displayed at too below position.

Comment 1

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

O.K.

Mihara-san, please attach the patch.
Let's go to 1.8.1(and 1.8.0.x if it's possible)!

Comment 2

[Katsuhiro MIHARA]

Attachment: change widget for calling LocalToWindowCoordinate() from focusedWidget to mTopLevelWidget.

textEvent.theReplay.mCursorPosition.{x,y,height} has mTopLevelWidget's coordinates. So we must call LocalToWindowCoordinate() to mTopLevelWidget.

Comment 3

[Katsuhiro MIHARA]

Attachment: use TAB as indent

nsMacEventHandler.cpp uses TAB as indent.

Comment 4

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

This bug is serious usability bug for IME users.
We should block next release.
# The patch has very low risk.

Comment 5

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

Comment on attachment 207866 [details] [diff] [review]
use TAB as indent

Jshin:

Could you review it?
# Asaf Romano is away until Feb 2006.

Comment 6

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

jshin:

Please review this. We hope that this will go to 1.8.0 branch if it's allowed.

Comment 7

[Jungshik Shin]

Comment on attachment 207866 [details] [diff] [review]
use TAB as indent

I'm afraid I'm not qualified to review this patch.

Comment 8

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 207866 [details] [diff] [review]
use TAB as indent

it's been so long, i'm not either. I know josh has been pouring over this code recently, passing to him.

Comment 9

[Josh Aas]

Comment on attachment 207866 [details] [diff] [review]
use TAB as indent

I assume the situation you're trying to correct is when

gEventDispatchHandler.GetActive()

returns something other than mTopLevelWidget and that becomes the focusedWidget. This patch does look like the right thing to do. Also, this is definitely one instance in which tabs are better than spaces (the whole file is indented with tabs!). r=josh

Comment 10

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

checked-in to Trunk.

Comment 11

[Hiro]

VERIFIED.

And attachment 207866 [details] [diff] [review] was tested with Thunderbird of 1.8branch.
This doesn't have the problem either. 
 
Mac OS X 10.3.9
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060215 Firefox/1.6a1

Comment 12

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

checked-in to 1.8 branch too.
Thank you for your testing.

Comment 13

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

Comment on attachment 207866 [details] [diff] [review]
use TAB as indent

Let's go to 1.8.0.x. This is critical bug for IME users.

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 207866 [details] [diff] [review]
use TAB as indent

approved for 1.8.0 branch, a=dveditz for drivers

Comment 15

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

checked-in to 1.8.0 branch too. Thanks.

Comment 16

[Hiro]

VERIFIED.

Thanks!!

1.8/1.8.0branch build.
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.2) Gecko/20060317 Firefox/1.5.0.2
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20060317 Firefox/2.0a1

Comment 17

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

Thank you for your test.