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

VERIFIED FIXED in mozilla1.8.1

Status

()

Core
Internationalization
P1
normal
VERIFIED FIXED
12 years ago
7 years ago

People

(Reporter: Katsuhiro MIHARA, Assigned: Katsuhiro MIHARA)

Tracking

(5 keywords)

Trunk
mozilla1.8.1
PowerPC
Mac OS X
inputmethod, intl, jp-critical, verified1.8.0.2, verified1.8.1
Points:
---
Bug Flags:
blocking1.8.0.2 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tjp-dl])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

12 years ago
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.
Assignee: nobody → smontagu
Status: UNCONFIRMED → NEW
Component: Layout: Form Controls → Internationalization
Ever confirmed: true
Keywords: intl
QA Contact: layout.form-controls → amyy
Assignee: smontagu → katsuhiromihara
O.K.

Mihara-san, please attach the patch.
Let's go to 1.8.1(and 1.8.0.x if it's possible)!
Priority: -- → P1
Target Milestone: --- → mozilla1.8.1
(Assignee)

Comment 2

11 years ago
Created attachment 207831 [details] [diff] [review]
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.
(Assignee)

Updated

11 years ago
Attachment #207831 - Flags: review?(bugs.mano)
(Assignee)

Comment 3

11 years ago
Created attachment 207866 [details] [diff] [review]
use TAB as indent

nsMacEventHandler.cpp uses TAB as indent.
Attachment #207831 - Attachment is obsolete: true
Attachment #207831 - Flags: review?(bugs.mano)
(Assignee)

Updated

11 years ago
Attachment #207866 - Flags: review?(bugs.mano)
This bug is serious usability bug for IME users.
We should block next release.
# The patch has very low risk.
Status: NEW → ASSIGNED
Flags: blocking1.8.1?
Comment on attachment 207866 [details] [diff] [review]
use TAB as indent

Jshin:

Could you review it?
# Asaf Romano is away until Feb 2006.
Attachment #207866 - Flags: review?(bugs.mano) → review?(jshin1987)
jshin:

Please review this. We hope that this will go to 1.8.0 branch if it's allowed.
Keywords: jp-critical

Comment 7

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

I'm afraid I'm not qualified to review this patch.
Attachment #207866 - Flags: superreview?(sfraser_bugs)
Attachment #207866 - Flags: review?(mikepinkerton)
Attachment #207866 - Flags: review?(jshin1987)
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.
Attachment #207866 - Flags: review?(mikepinkerton) → review?(joshmoz)

Comment 9

11 years ago
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
Attachment #207866 - Flags: review?(joshmoz) → review+

Updated

11 years ago
Attachment #207866 - Flags: branch-1.8.1+

Updated

11 years ago
Attachment #207866 - Flags: superreview?(sfraser_bugs) → superreview+
checked-in to Trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Comment 11

11 years ago
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
Status: RESOLVED → VERIFIED
checked-in to 1.8 branch too.
Thank you for your testing.
Flags: blocking1.8.1?
Keywords: fixed1.8.1
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.
Attachment #207866 - Flags: approval1.8.0.2?
Flags: blocking1.8.0.2?
Comment on attachment 207866 [details] [diff] [review]
use TAB as indent

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #207866 - Flags: approval1.8.0.2? → approval1.8.0.2+
Flags: blocking1.8.0.2? → blocking1.8.0.2+
checked-in to 1.8.0 branch too. Thanks.
Keywords: fixed1.8.0.2

Updated

11 years ago
Whiteboard: [tjp-dl]

Comment 16

11 years ago
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
Keywords: verified1.8.0.2, verified1.8.1
Thank you for your test.

Updated

11 years ago
Keywords: fixed1.8.0.2

Updated

11 years ago
Keywords: fixed1.8.1
Keywords: inputmethod
You need to log in before you can comment on or make changes to this bug.