339346 - Proper CJK input in Flash impossible (NS_KEY_DOWN events sent during IME input)

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Mac, defect)

Description

[Mark Mentovai]

Split from bug 212665 comments 2-3 to handle specifically the regression reported in those comments introduced by the series of event handling changes in bug 332579, bug 337199, and friends.

Testcase: switch input method to Hiragana, load a page with a Flash movie that accepts text input (such as http://www.weather.com/), and attempt to type into the movie.  Expected behavior: bottomline input window handles Hiragana input, and no text appearing in the Flash movie.  Observed behavior: text appears in the Flash movie corresponding to the Roman keys pressed.

Note that widget/src/cocoa (and Camino) shares this bug.  The new Carbon widget key event handling is very much like the implementation in Cocoa widgets.  I won't attempt to solve this bug for Cocoa here, we'll do another bug for that (although I suspect one is already open - we know we've got differences in IME between Carbon and Cocoa).

The source of this bug is that Flash is accepting NS_KEY_DOWN events to produce text input.  (I consider that erroneous.  Flash should use NS_KEY_PRESS for this purpose, like we do.  Oh well.)

In bug 337199 comment 21, I said of the new Carbon key event handling:
> One difference is that any keypress, even those in IME sessions, now
> produces an NS_KEY_DOWN event.  If in IME, there's no NS_KEY_PRESS, of
> course.  In the past, there wouldn't have been any NS_KEY_DOWN events when
> in IME either, but there would have been NS_KEY_UP events.  I don't think
> that the new behavior is wrong.  (But I might be wrong.)

Feedback Wanted!  Because IME is broken with Flash (or plugins in general) anyway, I don't consider this a major bug.  Even on the 1.8.0 branch (Firefox 1.5.0.x), where the testcase above doesn't produce Roman characters in the Flash movie, Flash does not accept the Hiragana text upon pressing RETURN.  If this new regression merely adds a new twist to things that were already broken under bug 212265, I don't consider it to be such a pressing issue.  However, if someone can provide a real testcase in which something used to work properly (even in the presence of bug 212265) and now doesn't, I'll bump the severity up.

Comment 1

[Mark Mentovai]

Proposed (unimplented, untested) solution to this bug I cooked up in the elevator, and one we can mimic in the Cocoa world:

In HandleKeyUpDownEvent, store the NS_KEY_DOWN event in a member of nsMacEventHandler without dispatching it.  Dispatch the stored event from HandleUKeyEvent prior to dispatching any NS_KEY_PRESS events.  HandleUKeyEvent is not called during an IME session, so NS_KEY_DOWN events corresponding to keys that were handled by IME will silently be dropped.  This should mimic the old behavior as far as NS_KEY_DOWN is concerned.

For congruity, I would avoid dispatch of NS_KEY_UP events when IME is active, because if we aren't dispatching NS_KEY_DOWN in that case, I think it's incongruous and buggy to dispatch NS_KEY_UP.  This would be a behavior difference from the old world (1.8.0 branch and earlier), but shouldn't be a quirk anyone relies on.

Comment 2

[Hiro]

The input of Japanese is a possible test case before. 
This problem still exists though Flash9 that had been released today was installed. 

http://bugzilla.mozilla.gr.jp/attachment.cgi?id=2275

Comment 3

[Mark Mentovai]

Cc Michelle

Comment 4

[Michelle Sintov]

Attachment: AddReturnHTML.html

Comment 5

[Michelle Sintov]

Attachment: AddReturnHTML.swf

Comment 6

[Michelle Sintov]

Here's how to see the Flash Player IME working in Mac Firefox 1.5:

1. In System Preferences, International Settings, set Japanese as the top option.
2. Log out and log in.
3. Launch Firefox 1.5.
4. Open AddReturnHTML.html, attached to this bug. This HTML file contains AddReturnHTML.swf, another attachment to this bug.
5. Click in the top Flash textfield.
6. Switch your IME by clicking on the US Flag in the upper right corner and select the 2nd option, Hiragana.
7. In that Flash top textbox, hit the 'a' key (notice a floating input window will show the Japanese character at the bottom left corner of the screen).
8. Hit return key to transfer the Japanese character to the Flash textfield.

Expected results: Japanese character will appear in the Flash textfield.

Comment 7

[Hiro]

(In reply to comment #6)
> Here's how to see the Flash Player IME working in Mac Firefox 1.5:
>
>7. In that Flash top textbox, hit the 'a' key (notice a floating input window
>will show the Japanese character at the bottom left corner of the screen).
>8. Hit return key to transfer the Japanese character to the Flash textfield.
>
>Expected results: Japanese character will appear in the Flash textfield.

In trunk build, Japanese character and 'a' appear. 
("aあ" appears. )

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060714 Minefield/3.0a1

Comment 8

[Mark Mentovai]

That makes this a regression.  I'm going to see if I can make something work along the lines of comment 1.

(But should Flash be producing text on NS_KEY_DOWN events?)

Comment 9

[Mike Connor [:mconnor]]

Not going to block on this, will consider a low risk patch, but the severity here is not high enough.

Comment 10

[Mark Mentovai]

Attachment: Patch (doesn't do the trick)

The proposed solution doesn't work.  Well, it works fine for Firefox, but it doesn't fix the Flash problem.  The problem is that Flash sets its own TSM document, so with this patch, we never reach our own HandleUKeyEvent to dispatch NS_KEY_DOWN or NS_KEY_PRESS events.

(Again:) why is Flash using NS_KEY_DOWN for text input in the way that it does?  I've never really been able to figure out exactly how Flash decides what's happening based on the combination of Gecko and native events, but it seems that it's considering NS_KEY_DOWN, NS_KEY_PRESS, and IME.

Comment 11

[Michelle Sintov]

From the plug-in code perspective (assuming that NS_KEY_DOWN is the same as the keyDown event we process from NPP_HandleEvent's event parameter), the Flash Player has been processing the keyDown event forever. We process both keyDown and keyUp.

When the floating input window is used, in Firefox 1.5 the Flash Player doesn't receive any key events until the user hits return on that floating input window. At that point, the Flash Player receives all the keyDown and keyUp events, and those are the events we process.

We do not adjust our handling of keyUp and keyDown when an IME is used. If we should, that is something we will need to investigate.

Let me know if I've misunderstood the question.

Comment 12

[Mark Mentovai]

Attachment: Patch (does do the trick)

OK, here's the game plan:

Before sending NS_KEY_DOWN events (which carry the keyDown event that you get in NPP_HandleEvent), I'll check to see if the active TSM doc is the same as the TSM doc that we use to handle text input for the window.  If it's not the same, then I assume that it's because some plugin activated its own TSM doc.  In that case, I'll try to dispatch to a TSM handler first, before sending the event internally through Gecko.  If the plugin's TSM handler does something with the event, then great - I'll stop, no further dispatch.  If the plugin's handler doesn't do anything, then I'll do the NS_KEY_DOWN thing so that the plugin can pick up the keyDown event, and I won't attempt further dispatch.

Even so, because of other architectural changes, the Flash plugin doesn't get autoKey events.  It might be best if the Flash player did all of its text input on its TSM handler instead of taking keyDown events.  keyDown events are for raw keyboard activity, and as you've seen, it's possible to get keyDown events that have nothing to do with the text the user is entering when an input session is active.

Michelle, if you do Firefox builds and want to test this patch, I'd appreciate your thoughts.

I have tested this patch with both Flash and Java.  I noticed that Flash IME stops functioning after loading Java.  I'll file a bug on this in a moment.

Comment 13

[Mark Mentovai]

Bug 347391 filed for Flash IME failure after loading Java in the same window.

Comment 14

[Mark Mentovai]

Adjusting title to better reflect the symptom, not just the cause.

Comment 15

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 232151 [details] [diff] [review]
Patch (does do the trick)

sr=pink

bleh??

Comment 16

[Mark Mentovai]

Oops.  "bleh" = "this accounts for suckage that may not be apparent, please write a better comment."

Comment 17

[Mark Mentovai]

Checked in on the trunk.

Actually, I just took the HandleKeyModifierEvent out.  I had only included it for congruity, but it wasn't really congruous: it suppressed more than HandleKeyUpDownEvent did.

Comment 18

[Mark Mentovai]

Comment on attachment 232151 [details] [diff] [review]
Patch (does do the trick)

Comment 9: will consider a patch.  Regression from 1.8.0.  Now baking.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

nsMacEventHandler makes drivers all nervous; can we get a volunteer to start running trunk as their primary browser and make sure this isn't causing any other oddities or regressions?

Comment 20

[Hiro]

In test case of comment#2 and comment#5, Firefox doesn't have the problem. 
But In Camino, there is still a problem. 

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060824 Minefield/3.0a1
Trunk Camino 2006082422 (v1.2+)

Comment 21

[Mike Connor [:mconnor]]

Comment on attachment 232151 [details] [diff] [review]
Patch (does do the trick)

a=mconnor on behalf of drivers for checking to the 1.8 branch

Comment 22

[Mark Mentovai]

Checked in on MOZILLA_1_8_BRANCH before 1.8.1rc1.

Comment 23

[Hiro]

(In reply to comment #20)
> In test case of comment#2 and comment#5, Firefox doesn't have the problem. 
> But In Camino, there is still a problem. 

Because default widget had become Cocoa by bug326469, Japanese was not able to be input with 
Firefox again.

Mac OS X 10.3.9
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20061021 Minefield/3.0a1

Comment 24

[Mark Mentovai]

As it happens, this bug is fixed.  If none of the existing bugs on Cocoa key and text events fits the bill, we should have a new one to track this issue.

Comment 25

[Hiro]

(In reply to comment #24)
> As it happens, this bug is fixed.  If none of the existing bugs on Cocoa key
> and text events fits the bill, we should have a new one to track this issue.
> 

It reported on bug357670 about the problem of Cocoa.