610015 - Need a way to check the composed text window (IME) is currently in use to decide what to do with backspace/arrow keystrokes

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Sunil]

Note: This bug requires a pre-release version of Silverlight that has Cocoa
event model enabled for FF4. 

We handle several keys ourselves for various purposes, particularly in text handling, to delete characters, move the cursor, and the like.    The IME window also handles those same keystrokes when it’s composing text.   We need to know, at key-down time, whether there’s a current composition window present, in order to decide what to do with the keystroke.

Example:  In a Silverlight Text box, someone enters “ABCDEFG”, then switches their keyboard to one of the Japanese Kana text entry keyboards, and starts entering “Mitsubishi”, which brings up the IME box to compose kana.     After a few characters, they make a mistake, say “Mitsw” and hit the backspace key.

What happens right now is:  We detect the keystroke, and remove the “G” from the current text box, and return “NPCocoaEventStartIME” as well, which causes the IME to delete the last character from the IME box as well.

If we switch to NOT returning “NPCocoaEventStartIME”, then there’s no way to delete bad characters in a composition.

If we don’t handle the key ourselves, the IME works fine but there’s no way to get a backspace in non-IME text, since that key won’t generate a corresponding NPCocoaEventTextInput event.   

So we need to change our return behavior (and whether or not we handle it) based on whether there’s a current composition or not.   The same logic applies to cursor keys, ESC, delete, and a few other keystrokes:  they mean something to both us and the IME, and we need a way to determine who should get them.

Ideally, we’d get a flag on each keystroke telling us whether the composition window is present or not (so we wouldn’t have to save state from key to key), but just knowing when the composition window opens and closes could allow us to determine this ourselves, with more work.

Comment 1

[Josh Aas]

The current and still evolving proposal to for an NPAPI fix is here:

https://wiki.mozilla.org/NPAPI:CocoaCompositionFlag

Comment 2

[Josh Aas]

Attachment: fix v0.5

This patch implements the proposed spec for 64-bit Mac OS X builds only and it isn't well tested. Just saving my work.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix v0.6 (work in progress)

Here's a start on (also) implementing the spec for 32-bit plugins
(I've added code to Josh's v0.5 patch).

But it doesn't (yet) work with OOP 32-bit plugins -- for that I need
to figure out how to hook the IMKInputSession methods in the
child/plugin process, then pass the requisite information to
nsChildView in the parent process.

I'll continue working on this next week.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix v1.0

Here's a revision of my v0.6 patch that seems to do everything needed
to fix this bug.

> But it doesn't (yet) work with OOP 32-bit plugins

This turns out not to be true.  I was confused by the behavior of the
tree's test plugin:  It doesn't support IME input in Cocoa event mode
(because it doesn't support the protocol described at
https://wiki.mozilla.org/NPAPI:CocoaEventModel#Text_Input).  And it
now (by default) uses the Cocoa event mode both when compiled 32-bit
and compiled 64-bit.

Tests with my DebugEventsPlugin (bug 441880, 32-bit only), which does
support the above-mentioned text input protocol (in Cocoa event mode)
work just fine.

> for that I need to figure out how to hook the IMKInputSession
> methods in the child/plugin process, then pass the requisite
> information to nsChildView in the parent process.

This wasn't needed, and in fact doesn't work.  All IME processing
takes place in the parent process.

But I did make one significant change from my v0.6 patch -- in
[ChildView keyDown:], I moved the TSMProcessRawKeyEvent() block (used
to support plugin IME in 32-bit code) before the test to find out
whether or not we're in IME composition.

It's the call to TSMProcessRawKeyEvent() that (in 32-bit code, if need
be) triggers an IME session.  So if it's called after the test (as it
used to be), mGeckoChild->GetInIMEComposition() returns the wrong
answer for the first and last keystrokes of an IME composition:  The
first keystroke of a composition (the one that brings up the input
window) is sent to the plugin with
cocoaEvent.data.key.compositionInProgress == FALSE (when it should be
TRUE).  And the last keystroke (typically Enter or ESC) is sent to the
plugin with cocoaEvent.data.key.compositionInProgress == TRUE (when it
should be FALSE).

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Here's a a tryserver build made with my patch from comment #4, with
some additional debug logging:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-30d556eb99b3/try-osx64/firefox-4.0b10pre.en-US.mac64.dmg

Please try it, Sunil, and let us know your results.

Comment 6

[Josh Aas]

I think the spec we're going to go with, which is currently in the proposal stage, is this:

https://wiki.mozilla.org/NPAPI:CocoaCompositionClarification

Sorry for the confusion Steven - I simply meant to request a mechanism for finding out the composition state for TSM documents. I have a patch for the rest of the proposed spec and I will post it soon.

Sunil - there was some confusion about specs, what Steven implemented is an old spec. No need to test that build.

Comment 7

[Josh Aas]

Attachment: fix v1.1

This patch implements the current proposal for Cocoa NPAPI input clarification. The spec isn't final so this patch might have to change but there is a good chance it won't change much so I'd like to start the review process.

Steven - Can you test this on 10.5?

Comment 8

[Josh Aas]

Attachment: fix v1.2

The spec was finalized and updated NPAPI headers have been committed to npapi-headers and mozilla-central. This patch implements the final spec.

Comment 9

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 504964 [details] [diff] [review]
fix v1.2

I see no problems with the patch code.  And I had no problems testing
it (with my DebugEventsPlugin from bug 441880) on OS X 10.6.5 (32-bit
and 64-bit) and 10.5.8 (32-bit only).

But npapi.h is missing from the patch (I supplied my own based on
Josh's earlier patches).

And I noticed a couple of syntax errors in nearby code -- the first
line of each of these methods ends in a semicolon, which it shouldn't:

> - (NPEventModel)pluginEventModel;
> {
>  return mPluginEventModel;
> }
>
> - (NPDrawingModel)pluginDrawingModel;
> {
>  return mPluginDrawingModel;
> }

Josh's patch might be a convenient place to fix these syntax errors.

Odd that Apple's gcc doesn't complain ... but it's been that way for
as long as I can remember (close to 10 years).

Note that the first character of a composition still gets fed to a
Cocoa-mode plugin.  But this is needed to give the plugin a chance to
return kNPEventStartIME or not (and thereby to veto the composition or
not).

Comment 10

[Josh Aas]

Thanks Steven. I'll included fixes for those syntax issues. I landed the NPAPI header changes last night which is why they aren't in this patch.

Comment 11

[Josh Aas]

Attachment: fix v1.3

Comment 12

[Josh Aas]

Attachment: fix v1.4

Fix some key up behavior - don't send any key up events associated with text input mode.

Comment 13

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/3d49b553bc4b