Closed Bug 447635 Opened 16 years ago Closed 16 years ago

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

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla1.9.1a2

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: inputmethod, intl, jp-critical)

Attachments

(1 file, 1 obsolete file)

Attached patch Patch v1.0 (obsolete) — Splinter Review
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?
Attached patch Patch v1.0.1Splinter Review
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?
Attachment #330942 - Flags: review?(smichaud)
Steven? Would you review this?
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.
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
Closed: 16 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?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: