IME candidate list window doesn't prefer the focused window level

RESOLVED FIXED in mozilla1.9.1a2

Status

()

Core
Widget: Cocoa
--
major
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({inputmethod, intl, jp-critical})

Trunk
mozilla1.9.1a2
x86
Mac OS X
inputmethod, intl, jp-critical
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 330940 [details] [diff] [review]
Patch v1.0

This is spun off from bug 433340.

On mac, we need to notify the current window level to IME via kTSMDocumentWindowLevelPropertyTag. However, we don't notify it.

And for doing it, we need to get the "actual" focused widget. The topmost <popup> element is not handling the key/IME events itself. Because it never has "native" focus. Therefore, we need to query the "actual" focused widget via NS_QUERY_TEXT_CONTENT event.

Note that the kTSMDocumentWindowLevelPropertyTag should be set at every focus changed. However, the <popup> element never gets the native focus. Therefore, this patch set the property at every keydown event (only IME is enabled).
Attachment #330940 - Flags: superreview?(roc)
Attachment #330940 - Flags: review?(roc)
Attachment #330940 - Flags: review?(joshmoz)
Flags: wanted1.9.1?
Flags: wanted1.9.0.x?
Created attachment 330942 [details] [diff] [review]
Patch v1.0.1

fix a mistake, sorry for the spam.
Attachment #330940 - Attachment is obsolete: true
Attachment #330942 - Flags: superreview?(roc)
Attachment #330942 - Flags: review?(roc)
Attachment #330942 - Flags: review?(joshmoz)
Attachment #330940 - Flags: superreview?(roc)
Attachment #330940 - Flags: review?(roc)
Attachment #330940 - Flags: review?(joshmoz)
oops,

s/<popup>/<panel>/

in comment 0.
I tested on following IMEs:

on 10.5
* ATOK 2006
* Kotoeri
* egbridge Universal 2
* ITABC
* Zhuyin
* Pinyin
* Cangjie
* Jianyi
* Dayi(Pro)

on 10.4
* ATOK 2006
* Kotoeri
* egbridge Universal
* ITABC
* Hanin
* Cangjie
* Jianyi
* Dayi(Pro)
* Pinyin
Attachment #330942 - Flags: superreview?(roc)
Attachment #330942 - Flags: superreview+
Attachment #330942 - Flags: review?(roc)
Attachment #330942 - Flags: review+
Josh:

Would you review this?

Updated

10 years ago
Attachment #330942 - Flags: review?(smichaud)
Steven? Would you review this?

Updated

10 years ago
Attachment #330942 - Flags: review?(joshmoz) → review+
Josh:

Do still I need the Steven's review?
Sorry for the delay.  I've been (and still am) on vacation.

I'll get to this on Monday (8-11).  Since the patch is a bit
complicated, I feel obliged to do some manual testing.

Comment 8

10 years ago
Lets move ahead without Steven's review, but leave the review request there. I'd like to have him look even if it happens after something lands.
pushed to trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: wanted1.9.1?
Resolution: --- → FIXED
Target Milestone: mozilla1.9.1 → mozilla1.9.1a2
Comment on attachment 330942 [details] [diff] [review]
Patch v1.0.1

Looking through the code, I have only one comment:

I suspect 'sDocumentID = nsnull;' shouldn't have been dropped from
nsTSMManager::EndComposing().  But (as best I can tell) leaving this
in or dropping it makes no difference in current code, so this is just
a minor quibble.

Otherwise everything looks fine.

I tested (on OS X 10.5.4 and 10.4.11) with the Japanese Hiragana and
Chinese Pinyin input methods, in combination with whatever oddball use
of popup windows and menus (native and non-native) I could think of.
I didn't see any problems.
Attachment #330942 - Flags: review?(smichaud) → review+
Steven:

Thank you for the review. I'll post the patch for the issue. For the safety, I agree your suggestion.
Flags: wanted1.9.0.x?
Keywords: inputmethod
You need to log in before you can comment on or make changes to this bug.