357670 - IME doesn't work on the text field of flash with Cocoa build

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P1)

Description

[Hiro]

Japanese cannot be input to the text field of Flash by a browser that uses Cocoa widget. 
(Oldest build(Camino 2005010108) that existed in the FTP server reproduced. )
Therefore, I originally think that this problem existed in Cocoa widget. 

In trunk Firefox, it reproduces by changing bug326469. 

Reproducible: Always

Mac OS X 10.3.9

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

You can find older Camino builds at http://archive.mozilla.org/pub/camino/nightly/

I sort-of expected that this would have already been filed against Camino at some point in the past if it had always been broken, but maybe not.

Comment 2

[Stuart Parmenter]

does this happen in firefox as well on the trunk?

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

yeah, I can reproduce this on trunk.

Comment 4

[Michelle Sintov]

If you follow the instructions to using the IME with Flash provided originally in bug 333881, I see the IME working in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1 on Mac OS 10.4.9. So presumably the Cocoa widget and the Flash Player aren't getting along, unfortunately.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Probably, this is a regression of bug 183313.

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Bug 183313 was fixed on 2005-10-13, while Hiro can reproduce the bug as far back as 2005-01-01....

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Flash 9 with Safari doesn't work fine for IME, I can use IME, but the garbage text is inputted...

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I found following code in Gecko 1.8:
http://lxr.mozilla.org/mozilla1.8/source/widget/src/mac/nsMacEventHandler.cpp#2259
> 2258 PRBool
> 2259 nsMacEventHandler::HandleKeyUpDownEvent(EventHandlerCallRef aHandlerCallRef,
> 2260                                         EventRef aEvent)
> 2261 {

> 2272   if (eventKind == kEventRawKeyDown) {
> 2273     if (IsPluginFocused()) {
> 2274       if (mTSMDocument != ::TSMGetActiveDocument()) {
> 2275         // If some TSM document other than the one that we use for this window
> 2276         // is active, first try to call through to the TSM handler during a
> 2277         // keydown.  This can happen if a plugin installs its own TSM handler
> 2278         // and activates its own TSM document.
> 2279         // This is done early, before dispatching NS_KEY_DOWN because the
> 2280         // plugin will receive the keyDown event carried in the NS_KEY_DOWN.
> 2281         // If an IME session is active, this could cause the plugin to accept
> 2282         // both raw keydowns and text input from the IME session as input.
> 2283         err = ::CallNextEventHandler(aHandlerCallRef, aEvent);
> 2284         if (err == noErr) {
> 2285           // Someone other than us handled the event.  Don't send NS_KEY_DOWN.
> 2286           return PR_TRUE;
> 2287         }
> 2288 
> 2289         // No foreign handlers did anything.  Leave sendToTSM false so that
> 2290         // no subsequent attempts to call through to the foreign handler will
> 2291         // be made.
> 2292       }

It sends the IME(TSM) event to next handler. But I don't know how to write the same code for Cocoa.

Comment 9

[Michelle Sintov]

The Flash Player team has not witnessed garbage characters in Safari. Could you tell me how you are using the IME? Also, could you tell me which version of the Flash Player you are using? Thanks.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #9)
> The Flash Player team has not witnessed garbage characters in Safari. Could you
> tell me how you are using the IME? Also, could you tell me which version of the
> Flash Player you are using? Thanks.

I tested with Flash Player 9,0,47,0 with Safari 2.0.4 (419.3) on MacOS X 10.4.10 (Ja).

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #10)
> (In reply to comment #9)
> > The Flash Player team has not witnessed garbage characters in Safari. Could you
> > tell me how you are using the IME? Also, could you tell me which version of the
> > Flash Player you are using? Thanks.
> 
> I tested with Flash Player 9,0,47,0 with Safari 2.0.4 (419.3) on MacOS X
> 10.4.10 (Ja).

er, it is Intel CPU.

Comment 12

[Michelle Sintov]

Could you provide more specific information about which IME you are using? For example, which item do you select from the OS dropdown when you click on the flag in the upper right-hand corner of the screen?

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #12)
> Could you provide more specific information about which IME you are using? For
> example, which item do you select from the OS dropdown when you click on the
> flag in the upper right-hand corner of the screen?

I think that here is not good for discussing for Safari and Flash, I'll send email.

Comment 14

[Michelle Sintov]

We have reproduced the garbage characters in Safari with Flash Player 9r47. This bug is now fixed in an upcoming release. The fix may be in the build located here (and if it's not fixed here, then the build with the fix is coming later): http://labs.adobe.com/technologies/flashplayer9/

Comment 15

[Josh Aas]

Is the Safari garbage characters problem fixed in the just-released Shockwave Flash 9.0 r115? How about this bug?

Comment 16

[philippe (part-time)]

(In reply to comment #15)
> Is the Safari garbage characters problem fixed in the just-released Shockwave
> Flash 9.0 r115?

Based on the test case linked here, the garbage input in Safari is fixed.

> How about this bug?
No, no Japanese input is possible. 10.4.11 ppc.

Comment 17

[philippe (part-time)]

Inputting Japanese actually results in a crash.
STR
1. command-click the test url linked above
2. input some (Japanese) text (or try to...) and fail
3. close the tab with the flash movie
4. after a small delay: boom. Minefield (2007120416) & Camino trunk crash.
http://crash-stats.mozilla.com/report/index/ba604aab-a2d6-11dc-b52d-001a4bd43ed6?date=2007-12-05-02

From the Camino crash log:
0   libobjc.A.dylib          	0x90a400f8 objc_msgSend + 24
1   org.mozilla.camino       	0x00845378 nsPrintSession::Release() + 106108
2   org.mozilla.camino       	0x00845438 nsPrintSession::Release() + 106300
3   com.apple.Foundation     	0x92be3a04 _nsnote_callback + 180
4   com.apple.CoreFoundation 	0x90804fd4 __CFXNotificationPost + 368
5   com.apple.CoreFoundation 	0x907fd04c _CFXNotificationPostNotification + 684
6   com.apple.Foundation     	0x92bcde0c -[NSNotificationCenter postNotificationName:object:userInfo:] + 92
7   com.apple.AppKit         	0x938232ec -[NSWindow resignKeyWindow] + 340
8   com.apple.AppKit         	0x939cbac0 -[NSWindow _resignKeyFocus] + 60
9   com.apple.AppKit         	0x93785984 -[NSApplication sendEvent:] + 4124
10  com.apple.AppKit         	0x9377cdf0 -[NSApplication run] + 508
11  com.apple.AppKit         	0x9386d974 NSApplicationMain + 452

Comment 18

[Michelle Sintov]

The proper test case to use is located here: http://www.playercore.com/bugFiles/ime/imekrjp.swf

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I don't have no idea for this bug.

We don't send any native events to plugin at IME events. Because there are no native events for IME(TSM) on cocoa. When key is downed, the keyDown event is only occurred. We call interpretKeyEvents of super class in keyDown. Then interpretKeyEvents calls NSTextInput methods for IME. So, if we need to send native carbon event (EventRecord) to plugins, we need to generate it ourselves. However, there are no documents for TSM events of EventRecord. We need to know what events are needed by plugins.

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #19)
> I don't have no idea for this bug.

Oops, I have no idea for this.

Comment 22

[Josh Aas]

Masayuki, I'm reassigning this to Steven for now. If you can contribute though your help would be greatly appreciated.

Comment 23

[Steven Michaud [:smichaud] (Retired)]

Attachment: Preliminary patch

Here's a preliminary version of my patch for this bug.  (It's
preliminary because it doesn't yet work in Camino, and because it
needs more testing than I can give it.)

My basic strategy was to imitate how the WebKit supports IME in
plugins -- it uses an undocumented HIToolbox function called
TSMProcessRawKeyEvent() to short-circuit Cocoa IME while a plugin has
the keyboard focus, and pass the appropriate key events to the plugin
via a Carbon handler for the kEventTextInputUnicodeForKeyEvent event.

This strategy seems to only be necessary in plugins that (like the
Flash plugin) do Carbon-based IME (using Text Services Manager
functions).  It's not needed by plugins that (like the Java Embedding
Plugin) do Cocoa-based IME ... but it doesn't cause trouble with them,
either.

Here's a tryserver build made with this patch:
https://build.mozilla.org/tryserver-builds/2008-04-07_09:[e-mail]-bugzilla357670-prelim/[e-mail]

I've done minimal testing of it using Japanese IME (mostly Hiragana)
and Chinese IME.  Others will be much better equipped to test it than
I am (particularly in Korean).  Masayuki? :-)

I'd also really like to see it tested in other plugins that support
IME.  The only ones I'm aware of are Flash and the Java Embedding
Plugin ... but I'm sure there are others.  Anyone know what they are?

Comment 24

[Steven Michaud [:smichaud] (Retired)]

Here's a Flex test URL (from bug 418334 comment #5).  Hiragana IME seems to work fine with it (with my patch).  (To get to an editable text area, you need to navigate (in the left pane) to Form Elements/Text Area.)

http://examples.adobe.com/flex3/consulting/styleexplorer/Flex3StyleExplorer.html

Once again, I'd like to see more examples, particularly using plugins besides Flash and the Java Embedding Plugin.

Comment 25

[Steven Michaud [:smichaud] (Retired)]

Attachment: Better patch (simplified, works in Camino)

Turns out I had to make fairly substantial changes to get my patch to
work in both Minefield and Camino.  (I now attach my Carbon text-event
handler to the event dispatcher target, and I've stopped storing any
information in the WindowDelegate class (which isn't available to
Camino and other embedders).)

I tested the patch on the three URLs mentioned elsewhere in this bug,
in both OS X 10.5.2 and OS X 10.4.11 (in both Minefield and Camino):

http://www.playercore.com/bugFiles/ime/imekrjp.swf
http://examples.adobe.com/flex3/consulting/styleexplorer/Flex3StyleExplorer.html
http://www.playercore.com/bugfiles/146162/AddReturnHTML.html

Once again, I'd really like to have more testcases, using different
plugins.

Mostly everything seems to work correctly.  But there are still some
oddball problems ... which don't seem important compared to the
problems I've solved, but which are still annoying:

Remaining problems (that I'm aware of):

1) When (only in Minefield) the focus is in a Flash plugin, you can't
   select either of the Thai keyboards (both are greyed out in the
   Flags menu).

   I'm not sure why this happens, but I suspect that it's because,
   when the Flash plugin (running in Minefield) creates its own TSM
   document (using NewTSMDocument()), its single supported interface
   is of type kTextServiceDocumentInterfaceType (I found this out in
   gdb).

   This problem doesn't happen in Camino ... because (oddly) the Flash
   plugin never calls NewTSMDocument() when it's running in Camino.
   So I have the browser create a TSM document for use by plugins that
   (like Camino) don't create their own (IME won't work without an
   active, non-Cocoa TSM document).  When I create this TSM document I
   make it support a single interface of type
   kUnicodeDocumentInterfaceType.  I'm pretty sure this is why Camino
   doesn't have the disabled-Thai-keyboards problem.

2) If (only in Minefield, only on OS X 10.4.11) I load one of the
   Flash/Flex examples (listed above) while a particular keyboard is
   active in the browser, I can't change to a different keyboard for
   input while that plugin is focused.  In order to change the
   keyboard (or IME style) in a plugin, I have to change it in the
   browser and then reload the plugin.

   Once again I suspect the Flash plugin is somehow at fault ... but I
   don't know how.  I _really_ want to be able to test in some other
   kind of plugin (like presumably the Adobe plugin) that doesn't do
   it's own Cocoa-based IME (as the Java Embedding Plugin does).

Here's a tryserver build made with this patch:

https://build.mozilla.org/tryserver-builds/2008-04-09_16:[e-mail]-bugzilla357670/[e-mail]

Comment 26

[Steven Michaud [:smichaud] (Retired)]

Michelle, please please please get your colleagues at Adobe to provide
testcases for all the Adobe plugins :-)

Comment 27

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 314739 [details] [diff] [review]
Better patch (simplified, works in Camino)

Josh, I'm asking you to review, even though this patch probably isn't
in its final form (and though I _can't_ really finish it until I have
more URLs to test with).

Think of this as _starting_ the review process :-)

Comment 28

[Steven Michaud [:smichaud] (Retired)]

More about the Thai keyboards problem:

I've found that they don't work (they don't output Thai characters) when the focus is in a Flash plugin, in Camino _or even in Safari_ (on both 10.4.11 and 10.5.2).

So Flash is actually correct (running in Minefield) to disable them.

This must be a WebKit bug ... which bites us now that we're using the WebKit's strategy to support IME in plugins.

Comment 29

[Steven Michaud [:smichaud] (Retired)]

> This must be a WebKit bug ... which bites us now that we're using
> the WebKit's strategy to support IME in plugins.

Not just a WebKit bug -- it also happens in Firefox 2.0.0.13 (in a
Flash plugin).

Comment 30

[Steven Michaud [:smichaud] (Retired)]

> More about the Thai keyboards problem:

There are exactly parallel problems with the Arabic keyboard.

Neither keyboard uses an IME; both keyboards alter already "typed" characters depending on their context (depending on what you "type" subsequently).

Comment 31

[Josh Aas]

Comment on attachment 314739 [details] [diff] [review]
Better patch (simplified, works in Camino)

+  for (unsigned i = 0; i < numCharCodes; ++i) {

"unsigned int", full name please

+// Tag "our" Cocao TSM documents and those which belong to one of our plugin

"Cocoa"

+  id arp = [[NSAutoreleasePool alloc] init];

Instead of releasing this for each early return, can we use 'nsAutoRetainCocoaObject' or some variant that wouldn't double retain? You'd have to release the pool after creating the C++ object because you can't autorelease it and the C++ object calls retain. I don't care much one way or another whether you implement this suggestion since we don't do it anywhere else but it might make for cleaner and safer code.

+  OSStatus status = GetEventParameter...

Please prefix OS calls like this with "::" to make them visually stand out.

Comment 32

[Atsushi Sakai]

Attachment: Screenshot of garbled text

This screenshot shows when I typed "a".
This should be Hiragana "あ".

Reproducible: Always

Steps to reproduce:
1. Change to Hiragana mode.
2. Start Firefox.
3. Open a  page which contains the input field of Flash.
4. Make sure Hiragana works fine in Flash.
5. Change to Roman mode while Flash has focus.
   This can be done by pressing "英数" key on Japanese Mac keyboard.
   Or pressing Ctrl+Shift+; if the patch of Bug 423814 applied.
6. Change to Hiragana mode when Flash doesn't have focus.
   This can be done by click location bar or somewhere, change mode to Hiragana and click the Flash.
   Or this can be done by changing mode with the input menu in menu bar.
7. Input some text into the input field in Flash.

Actual results:
The text is garbled during transaction.

Expected results:
The text is NOT garbled during transaction.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008040916 Minefield/3.0pre

Comment 33

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #32)

That might be our bug...

Comment 34

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #33)
> (In reply to comment #32)
> 
> That might be our bug...

Oops, that might *not* be out bug.

Comment 35

[Atsushi Sakai]

(In reply to comment #34)
> Oops, that might *not* be out bug.

This problem does not occur with Safari.
If that is not our bug, whose bug?

Comment 36

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #35)
> (In reply to comment #34)
> > Oops, that might *not* be out bug.
> 
> This problem does not occur with Safari.
> If that is not our bug, whose bug?

Oh, really?

Comment 37

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #32)

I can't test your "steps to reproduce", since I haven't applied the
patch for bug 423814,

Which OS did you test on (10.5.X or 10.4.X)?  10.5 forces all
text-input fields (all "documents") to use the same keyboard and/or
IME (for example if you choose Hiragana while the keyboard focus is
anywhere in any browser window, all text-input fields in all window
start using Hiragana).  But 10.4 (by default) allows each NSView
object to be set to use a different keyboard.  So you might have the
standard parts of a browser window set to use the US keyboard, one
plugin (in the same window) set to use Hiragana, and	another plugin
(in the same window) set to use	Katakana.

Were you testing on OS X 10.4.X?  If so, please change the following
setting (in the International preference pane under the Input Menu
tab), and see if it makes your problem go away:

Change "Allow a different source for each input	source for each
document" to "Use one input source in all documents".

Comment 38

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #32)

Also, can you reproduce your problem in Firefox 2.0.0.13, or Camino 1.5.5, or a recent Camino trunk nightly?

Comment 39

[Michelle Sintov]

(In reply to comment #26)
> Michelle, please please please get your colleagues at Adobe to provide
> testcases for all the Adobe plugins :-)

Adobe Reader doesn't support Firefox on Mac. I have pinged the Shockwave team to see if they have support for IME, and if so, test URLs.

Comment 40

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #32)

Actually, I _am_ able to reproduce this, though only on OS X 10.4.X.
The one/many input source setting makes no difference.  I also see it
in Firefox 2.0.0.13.  I don't see it in Safari.  I don't see it
anywhere on OS X 10.5.X.

Though this doesn't happen in Safari on 10.4, it looks to me like an
Apple bug -- specifically a bug in the "input window" that shows up
when you do IME in a context that (like the Flash plugin) doesn't
support inline IME.  The bug seems to have been fixed on 10.5.

Here are clearer (and simpler) steps to reproduce, which work without
a Japanese keyboard or the patch for bug 423814.  But to make them
work you'll first need to open the Keyboard & Mouse pref panel, choose
Keyboard Shortcuts, turn off the Spotlight shortcuts and turn on the
Input Menu shortcuts.

1) Load http://www.playercore.com/bugFiles/ime/imekrjp.swf and put the
   keyboard focus into one of its text-input fields.

2) Command-Option-spacebar to cycle through the available
   keyboards/IMEs and choose Hiragana.

3) Type some text (say "hiragana"), but don't hit Enter.  The input
   window will contain the Hiragana for "hiragana", and will remain
   open.

4) While the input window is still open, Command-Option-spacebar to
   cycle through the available keyboards/IMEs until you reach Romaji.
   The input window will remain open.

5) Type a single letter in the open input window.  All the text in it
   will become garbled.  But (interestingly) if you hit Enter, the
   correct text will appear in the plugin's text-input field.

Comment 41

[Steven Michaud [:smichaud] (Retired)]

Attachment: Patch rev2 (follow Josh's suggestions, fix problems)

Here's a revision of my patch that follow's Josh's suggestions (most
of them) and also fixes problem #2 from comment #25.

> +  id arp = [[NSAutoreleasePool alloc] init];
>
> Instead of releasing this for each early return, can we use
> 'nsAutoRetainCocoaObject' or some variant that wouldn't double
> retain?  You'd have to release the pool after creating the C++
> object because you can't autorelease it and the C++ object calls
> retain. I don't care much one way or another whether you implement
> this suggestion since we don't do it anywhere else but it might make
> for cleaner and safer code.

To save time I decided not to do this.  Let's handle it during the
"code cleanup" phase ... what happens when you wake up after the wild
code-writing party and you realize there are a few things you've
forgotten, or would rather forget :-)

As for problem #1 from comment #25, I decided it's not a bug but a
feature.  Presumably no plugin that gets its keyboard events from the
NPAPI will be able to handle Thai or Arabic (or Hebrew) input
correctly, so it makes sense to grey out those entries in the Flags
menu.  So this patch now emulates the Flash plugin when it calls
NewTSMDocument(), and suports a single interface of type
kTextServiceDocumentInterfaceType.

I also dropped a bunch of unnecessary code that my previous patches
added to [ChildView keyUp:].

A tryserver build will follow shortly.

Comment 42

[Steven Michaud [:smichaud] (Retired)]

Here's a tryserver build made with what I've called my rev2 patch.  Yes, the URL says rev1 ... but it's the same patch.

https://build.mozilla.org/tryserver-builds/2008-04-10_16:[e-mail]-bugzilla357670-rev1/[e-mail]

Comment 43

[John Daggett (:jtd)]

Simple Japanese entry testcase for Josh:

1. Click on text entry field
2. Switch the input method to Hiragana, you'll see a small 'あ' appear in the input method menu.
3. Type this exact key sequence:

boku<space><return>ha<return>kare-<space><return>ga<return>daisuki<space><return>desu<return>!<return>

You should end up with:

僕はカレーが大好きです！

Comment 44

[John Daggett (:jtd)]

Attachment: screenshot, running with try server build

Using the try server build linked in comment 42, I see a couple problems:

(1) text displays in input method window rather than inline
(2) cursor place is screwed up - the screenshot shows the cursor positioned inside the string when in fact the insertion point is at the *end* of the string

Is there a reason we can't allow inline display (dare I ask)?

Comment 45

[John Daggett (:jtd)]

Safari shows behavior identical to the test build.

Safari 3.1 (4525.13) with Shockwave Flash 9.0 r115
Mac OS X 10.4.11 (8S2167)

Masayuki, are you really still using Safari *2*?  You old timer, you... ;)

Comment 46

[Steven Michaud [:smichaud] (Retired)]

> Is there a reason we can't allow inline display (dare I ask)?

As best I can tell, the only option for plugins that (like Flash) get their keyboard events via NPAPI is to use the input window.  But I'd really like to be able to test with more plugins.

> (2) cursor place is screwed up

I can't see anything from your screen shot.  Please be more specfic.

For example, what URL did you visit?  And I assume you were doing text input in a plugin (via the input window) ... but tell me if otherwise.

And give specific steps-to-reproduce.

> Safari shows behavior identical to the test build.

That's the best we can hope for, at least for now (prior to the Firefox 3 release).

Comment 47

[Steven Michaud [:smichaud] (Retired)]

> 僕はカレーが大好きです！

「カレー」わ何ですか？ What's karee? :-)

Comment 48

[John Daggett (:jtd)]

The cursor displays incorrect in the Flash text field, in the screenshot you can see it placed on top of the onbiki character, the third character in カレー and just before the が character.  It should be to the right of the xxx character, as you can see that I'm in the middle of entering new text down at the bottom of the screen in the input method window.

The testcase is the one described in comment 43.  The screenshot was taken just after the completion of this sequence of keystrokes:

boku<space><return>ha<return>kare-<space><return>ga<return>daisuki<space>

As for カレー, it is an ancient traditional Japanese food:

http://ja.wikipedia.org/wiki/%E7%94%BB%E5%83%8F:Curry_and_rice.jpg

Here's a recipe:

Japanese-style curry rice
Source: Noriko's Kitchen
Cooking time: 3 hours

Read the instruction on the box of commercial curry roux and find out
what ingredients you need first.  You may increase or decrease the
amount of vegetables and meat by as much as 50% if you wish.  However,
the amount of water you add should not be decreased.  If the consistency
of the curry stew is too thick, adjust it by adding water at the end.  The
commercial curry roux contains everything, so you do not need to add salt
or pepper.  Typical ingredients are listed below.

Ingredients:

1 medium yellow onion, sliced
2 or 3 potatoes, cut into big or small chunks
2 carrots, diced
1 cup whole mushrooms
up to 3/4 lb. beef or other meat, cut into bite-sized pieces
1 box curry roux (Japanese brand)
one serving freshly cooked warm rice

Directions:

Heat a deep pan and add one or two tablespoons of oil.  Saute the sliced
onion over medium heat until it softens.  Brown the meat separately, then
add it to the onion.  Add water as specified, add a bay leaf, and simmer
for 2 hours.  Then add the potatoes, carrots and mushrooms and continue
simmering.  When the potates and carrots are tender add the curry roux. 
Simmer gently for 15 minutes while stirring.  Serve with rice.

My favorite variation, カツカレー:

http://ja.wikipedia.org/wiki/%E7%94%BB%E5%83%8F:Katsukare-.jpg

Comment 49

[Steven Michaud [:smichaud] (Retired)]

I truly can't see anything in your screen shot.  Even thirty years ago
my eyes probably wouldn't have been up to it.

I also can't reproduce any cursor misalignment/mislocation issues with
your testcase from comment #43.  I tested in the "Japanese" text-input
area from http://www.playercore.com/bugFiles/ime/imekrjp.swf, with my
rev1/rev2 tryserver build on OS X 10.5.2 and 10.4.11.

And here I thought curry was a traditional British dish :-)

Thanks for the recipe, though!

Comment 50

[Steven Michaud [:smichaud] (Retired)]

> I truly can't see anything in your screen shot.  Even thirty years ago
> my eyes probably wouldn't have been up to it.

I just tried enlarging the image ... which was a bit easier on the eyes and makes it possible to see what you're talking about.

I'm definitely unable to reproduce that.

Comment 51

[Atsushi Sakai]

(In reply to comment #37)
> Which OS did you test on (10.5.X or 10.4.X)?

I'm testing on 10.5.2, not 10.4.X.
Please see UA string in comment #32.

(In reply to comment #40)
> Actually, I _am_ able to reproduce this, though only on OS X 10.4.X.

Comment #32 is reproducible on 10.5.
I think this is similler but different problem.

I didn't know Command-Option-spacebar shortcut, but this is useful to simplify the steps to reproduce.
Simplified steps to reproduce my problem:

1. Load http://www.playercore.com/bugFiles/ime/imekrjp.swf and put the
   keyboard focus into one of its text-input fields.

2. Command-Option-spacebar to cycle through the available
   keyboards/IMEs and choose Hiragana.

3. Type some text (say "hiragana") and hit Enter.
   Correct Hiragana text appears.

4. Command-Option-spacebar to cycle through the available
   keyboards/IMEs and choose Roman.

5. Put the keyboard focus into the Location Bar or the Search Bar.

6. Command-Option-spacebar to cycle through the available
   keyboards/IMEs and choose Hiragana.

7. Put the keyboard focus into one of plugin's text-input fields.

8. Type a single letter (say "a") which represents Hiragana, but don't
   hit Enter.  The input window will contain garbled text.

9-a. If you hit Enter, the correct text will appear in the
   plugin's text-input field.

9-b. If you type more letters, Roman will be choosed though I've
   chosen Hiragana. And when you hit Enter, the garbled text will
   appear in the plugin's text-input field.

With tryserver build mentioned in comment #42.

(In reply to comment #38)
> Also, can you reproduce your problem in Firefox 2.0.0.13, or Camino 1.5.5, or a
> recent Camino trunk nightly?

Firefox 2.0.0.13 can reproduce from 1 to 9-a above. 9-b does not occur.
Camino 1.5.5 and trunk can not reproduce because IM does not work in Flash.

By the way as for curry, it's good timing.
My dinner was curry and rice today:)

Comment 52

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 314996 [details] [diff] [review]
Patch rev2 (follow Josh's suggestions, fix problems)

Josh, there's a bizarre problem with this patch (and previous ones) on
Camino on 10.4.X.  I've got it fixed, but I'm now working on getting
out a new version of the JEP.

I should be able to put up a new patch later today ... or perhaps
tomorrow.

Comment 53

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #51)

I'm able to reproduce what you report, along with even worse problems
-- basically, Hiragana input in the "input window" can become unusable
until you restart the browser.

But I've got a new patch that appears to resolve both these problems
and the problem I reported in comment #40.

Comment 54

[Steven Michaud [:smichaud] (Retired)]

Attachment: Patch rev3 (fix problems with input window, simplify)

Here's a new version of my patch that (as far as I can tell) resolves
all problems with mixed input (e.g. Hiragana and Romaji) in the "input
window".

In order to accomplish this, I had to make my patch always use the TSM
document it creates for use with plugins (and with
TSMProcessRawKeyEvent()).  So even if a plugin creates its own TSM
document (as the Flash plugin does when running in Firefox), "our"
plugin-specific TSM document will preempt it.

The reason for this is that the only way I could find to get rid of
the "input window" problems was to create a TSM document that supports
both the kTextServiceDocumentInterfaceType and kUnicodeDocument
"services".  This is what the OS does when it creates a TSM document
for use by the NSTSMInputContext class.

A tryserver build will follow shortly.

> Josh, there's a bizarre problem with this patch (and previous ones)
> on Camino on 10.4.X.

This problem has disappeared ... without me ever being able to figure
out what caused it.  For the record (and in case it recurs), the
problem was as follows (only on OS X 10.4.x):

1) Start Camino and put the keyboard focus in the location bar.

2) Switch from the US keyboard to the Hiragana keyboard -- the arrow
   keys and delete keys stop working in the location bar.

Comment 55

[Steven Michaud [:smichaud] (Retired)]

Here's a tryserver build made with my rev3 patch:

https://build.mozilla.org/tryserver-builds/2008-04-14_08:[e-mail]-bugzilla357670-rev3/[e-mail]

Comment 56

[Steven Michaud [:smichaud] (Retired)]

Michelle, you'll have noticed that the latest version of my patch
always uses its "own" TSM document to support IME in plugins.  It does
this even when the plugin (like Flash running in Firefox) creates and
tries to make active its own TSM document.

As far as I can tell, this is both necessary and doesn't cause any
problems.  But can you forsee any problems, or find testcases that
show problems?

(It's necessary because (as I point out in comment #54 and in my patch
comments in [ChildView activatePluginTSMDoc]) the TSM document for
plugin use needs to have been created in a particular way -- to
support both the kTextServiceDocumentInterfaceType and
kUnicodeDocument "services", in that order.  Otherwise you get nasty
problems with the "input window".)

Comment 57

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 315562 [details] [diff] [review]
Patch rev3 (fix problems with input window, simplify)

> +OSStatus PluginKeyEventsHandler(EventHandlerCallRef inHandlerRef,
> +                                EventRef inEvent, void *userData)
> +{
> +  id arp = [[NSAutoreleasePool alloc] init];

NSAutoreleasePool is really needed here? I'm going to remove NSAutoreleasePool from insertText (see bug 428980). That can make easy leaking bug later.

Comment 58

[Steven Michaud [:smichaud] (Retired)]

> NSAutoreleasePool is really needed here?

I don't know for sure.  But it's always been my practice to create an
autorelease pool for a Carbon event handler, and I think we should
keep it (just to be safe).

Comment 59

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+// defined in nsChildView.mm
+extern void InstallPluginKeyEventsHandler();
+extern void RemovePluginKeyEventsHandler();

Can't these go in nsChildView.h? And really these should be namespaced somehow, even if it's just NS_.

+  // Transient (only set while TSMProcessRawKeyEvent() is processing a key
+  // event), so it can be weak.
+  kFocusedChildViewTSMDocPropertyTag  = 'GKFV', // type ChildView* [WEAK]

Are you sure the plugin can't reenter us and cause the destruction of ChildViews? I'm not.

Please make sure an Apple Radar bug is filed on making TSMProcessRawKeyEvent a public API. Maciej Stachowiak promised to prioritize such bugs, so you probably want to CC him if the Apple bug system allows that.

+  unsigned char *charCodes = new unsigned char[numCharCodes];

Use nsAutoArrayPtr, or better still nsAutoTArray<unsigned char> with SetLength() and Elements().

+      PRUint32 keyCode = GetGeckoKeyCodeFromChar((PRUnichar)charCodes[i]);
+      PRUint32 charCode = (PRUint32) charCodes[i];

Use constructor-style casts.

This is really sad but it sounds like we have to do it.

Comment 60

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It seems like we might want an nsRetainPtr<T> to avoid manual release calls, but that's grist for another bug.

Comment 61

[Steven Michaud [:smichaud] (Retired)]

Attachment: Patch rev4 (follow roc's suggestions)

> +  // Transient (only set while TSMProcessRawKeyEvent() is processing a key
> +  // event), so it can be weak.
> +  kFocusedChildViewTSMDocPropertyTag  = 'GKFV', // type ChildView* [WEAK]
>
> Are you sure the plugin can't reenter us and cause the destruction
> of ChildViews? I'm not.

I now retain and release the ChildView object around all calls that
set this property (in [ChildView keyDown:]).  But I've left this
pointer weak, because I don't want to have to worry about calls to
TSMProcessRawKeyEvent() that (for whatever reason) never result in
calls to the PluginKeyEventsHandler() handler.

If [ChildView keyDown:] _can_ be reentered, it's still possible that
some key events may get dropped (not sent to a plugin).  But I think
this is very unlikely -- we should deal with it if and when it
happens.

> +  unsigned char *charCodes = new unsigned char[numCharCodes];
>
> Use nsAutoArrayPtr, or better still nsAutoTArray<unsigned char> with
> SetLength() and Elements().

I've used nsAutoArrayPtr, because I need a flat array for use with
GetEventParameter().

Comment 62

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Elements() gives you a flat array.

Comment 63

[Steven Michaud [:smichaud] (Retired)]

Attachment: Patch rev5 (use nsTArray)

> Elements() gives you a flat array.

Oops, OK.

Comment 64

[Steven Michaud [:smichaud] (Retired)]

Oops again.  Need nsAutoTArray.  New patch coming up.

Comment 65

[Steven Michaud [:smichaud] (Retired)]

Attachment: Patch rev6 (use nsAutoTArray)

Comment 66

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 315834 [details] [diff] [review]
Patch rev6 (use nsAutoTArray)

This patch is moderately risky (it's not yet been tested with other
plugins than the Flash plugin and the Java Embedding Plugin).

But the benefit is high -- without it, IME isn't possible in Flash
"movies" or Flex objects (a regression from Firefox 2.0.0.X).

Comment 67

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

Comment on attachment 315834 [details] [diff] [review]
Patch rev6 (use nsAutoTArray)

a=beltzner, it would be good if we could get some more testing on this. Can you contact tchung and see if you can get some QA resource?

Comment 68

[Steven Michaud [:smichaud] (Retired)]

The testing this patch needs is with other plugins that support text
input (and, potentially, also IME input).

I honestly don't know whether there are any such plugins (at least
among those in common use).  But I'd really appreciate help finding
them (and finding testcases for them that use text input).

In the meantime I'll land this patch (tomorrow) ... which will also
help with the testing :-)

Comment 69

[Steven Michaud [:smichaud] (Retired)]

Landed on trunk:

Checking in widget/src/cocoa/nsAppShell.mm;
/cvsroot/mozilla/widget/src/cocoa/nsAppShell.mm,v  <--  nsAppShell.mm
new revision: 1.31; previous revision: 1.30
done
Checking in widget/src/cocoa/nsChildView.h;
/cvsroot/mozilla/widget/src/cocoa/nsChildView.h,v  <--  nsChildView.h
new revision: 1.90; previous revision: 1.89
done
Checking in widget/src/cocoa/nsChildView.mm;
/cvsroot/mozilla/widget/src/cocoa/nsChildView.mm,v  <--  nsChildView.mm
new revision: 1.334; previous revision: 1.333
done

Comment 70

[Dave Townsend [:mossop]]

Backed this out due to bug 434914

Comment 71

[Dave Townsend [:mossop]]

Attachment: backout patch

This was the patch used to backout, slightly different due to the presence of bug 433432 and bug 431503 which landed after this.

Comment 72

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oh, the backing out reopens too very important accessibility bug... We cannot type intl text on Flash again :-(

Mozilla Japan doesn't hope to release the final without this fix...

E.g., Japanese Mac users cannot use Ustream and NicoNico Doga.

Comment 73

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

Masayuki: it wasn't an easy decision, but see bug 434914 for the full rationale. Our choices were: break IME text input on Mac or break keyboard interaction on flash apps for everyone.

Fundamentally the fix here is flawed and wrong. It shouldn't be in our code. We can't block on this for release, and we'll try to fix it in a branch release and relnote this for now.

Comment 74

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #72)

I tend to agree with you that this bug is more important than bug
434914.  And I suspect a lot of the commenters there didn't test RC1,
and commented only because someone told them "Flash is broken on RC1".

But both bugs do inconvenience a large number of users, and it was a
difficult decision to make (whether or not to back out this bug's
patch).  Arguments can be made on both sides.

I'm going to (temporarily) drop everything else and work on a fix for
this bug ... in the expectation that political necessity may require
one before Firefox 3.0.1 :-)

I expect I'll need a lot of help from testers ... so be prepared :-)

Comment 75

[Steven Michaud [:smichaud] (Retired)]

> I'm going to (temporarily) drop everything else and work on a fix
> for this bug

Or rather, alterations to this bug's patch (and probably also to
OS-X-specific code in nsObjectFrame.cpp) that fix bug 434914.

Comment 76

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

(In reply to comment #73)
> rationale. Our choices were: break IME text input on Mac or break keyboard
> interaction on flash apps for everyone.

To be clearer: break IME text input on the Mac (well scoped, understood, contained) or break events within flash for everyone (depends on how flash app is written, unpredictable effects, not well contained). We did some quick scouring of the web and found that it's not just flash games which were effected, but things like keyboard shortcuts on YouTube Player.

It's not like we were happy to have to break this again.

Comment 77

[Mike Shaver (:shaver emeritus)]

We need more than "help from testers".  We need actual test coverage, and detailed analysis of what event inputs we get today (and with FF2), and what events are then passed to plugins.  And we need to compare that with other browsers.  Someone's going to have to write instrumentation to log events, and read those logs carefully.  Might need to write a tiny plugin, or you could probably get a long way with a small amount of AS in a SWF.

The time has long past for "let's try this and see if we can find regressions with a few URLs" -- we need to be systematic and thorough.  Otherwise we're going to be right back in this mess.

Comment 78

[Steven Michaud [:smichaud] (Retired)]

> Might need to write a tiny plugin, or you could probably get a long
> way with a small amount of AS in a SWF.

Both of these would be useful.  And we already have the beginnings of
the second at bug 434914 (attachment 322116 [details]).  I'll do both of these
before I try to land another patch.

But I think this is enough for our current purposes.  Both of these
can (presumably) be fit into whatever test framework we eventually
come up with (for Cocoa widgets and/or plugins).

Comment 79

[Steven Michaud [:smichaud] (Retired)]

Attachment: Patch rev7 (fix bug 434914)

Here's a patch that seems to fix bug 434914 without regressing this
bug (bug 357670).  Here's a tryserver build made with the patch:

https://build.mozilla.org/tryserver-builds/2008-06-02_13:[e-mail]-bugzilla357670-rev7/[e-mail]

Masayuki and/or Atsushi, please try out this build with the "Ustream
and NicoNico Doga" mentioned in comment #72, with the URLs from
comment #25, and with anything else you think appropriate.

Jim, please try this out with your company's games (I already tried
the dolphin olympics game from bug 434914 comment #2 believe I had no
trouble).

Ben, please confirm that your testcase (attachment 322116 [details]) from bug
434914 comment #4 works as expected with this patch (and tryserver
build).

Yes, I know I also need to provide an event-testing minimal plugin and
a more comprehensive Flash event tester.  I'm working on them.

Comment 80

[Steven Michaud [:smichaud] (Retired)]

Attachment: Version of rev7 patch against tree prior to backout (for illustration)

Comment 81

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Great, rev7 patch works fine for me on flash contents on the two sites, thank you.

Comment 82

[Atsushi Sakai]

Rev7 works fine for me, too.
With Kotoeri under OS X 10.5.

Comment 83

[Asahiko Matsuda]

Came here after reading about this bug on ITmedia News. I'm no programmer so Mr Nakano may not like a person like me being here, but I really would like to see this bug fixed 'cause Firefox would never be my primary browser if I can't post on NicoNico.

Rev 7 works great for me too, under Kotoeri (Japanese IME), OS X 10.5.3. Just superb.

Comment 84

[Ben Vinson]

Flash Keyboard events seem to be working fine as well, sorry about the slow response!

Comment 85

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Steven:

Let's land the patch to trunk, and the patch should be tested by nightly build testers before landing to 1.9.0 branch.

So, let's go to the review process.

Comment 86

[Mike Shaver (:shaver emeritus)]

Masayuki: can you help Steven with writing the necessary tests for this patch?  That will help get it landed, certainly.

Comment 87

[Steven Michaud [:smichaud] (Retired)]

Automated tests aren't feasible for this bug, or in general for
plugin-side tests on any platform (as far as I know).

So the only reasonable course is non-automated tests.

I'm currently working on a test NPAPI plugin for the Mac -- one that
records the events it receives, and other stuff besides.  It's been
more work than I anticipated, and I've lost a lot of time to other
things (bug 436575, general bug triage in the wake of the FF3
release).  But I'm almost done with it, and (fingers crossed) it will
be ready early next week.  (When it's done I'll post it to bmo so
everyone can use it.)

So yes, Masayuki, lets start the review process now.

I haven't yet started writing an event-testing Flash "movie" (a more
comprehensive one than attachment 322116 [details]).  Help with this would be
welcome, but not essential -- attachment 322116 [details] will be adequate for
testing this bug.

Comment 88

[Mike Shaver (:shaver emeritus)]

Why aren't automated tests feasible?  You need to synthesize a key event, and then make sure that the right thing happened, and it's quite possible that the test plugin could synthesize its own events and listen for the results.  Even if we don't have such a test setup running on a tinderbox right away, having a tool that people can run and get pass/fail out of when they change things related to event dispatch would probably keep us from falling back into our event hell quite so readily.

http://groups.google.com/group/mozilla.dev.quality/browse_thread/thread/1f3c4b46b738020d/8dc215fa5a0f1b6f#8dc215fa5a0f1b6f is an earlier thread in which it was discussed, and I took your reply there to mean that you agreed it to be a good course of action, so I'm surprised by your comment here!

Naively, I would suggest using CGEventCreateKeyboardEvent and CGEventPostToPSN to get started, but I will readily admit that I haven't tried them.  Were they attempted and found to not be suitable?  (The event-tap stuff would also be useful for dealing with cases that are hard for our Mac developers to reproduce, I imagine, but that's a second-order concern.)

Comment 89

[Steven Michaud [:smichaud] (Retired)]

> Why aren't automated tests feasible?

Because we currently have no automated way of telling what events
reach the plugin, and in which order.

> so I'm surprised by your comment here!

The context was (of course) completely different.

Håkan's first two items only make sense as part of an automated test
framework.  That's all I meant when I added my third item.

I'm not opposed to automated tests.  They're very convenient (in that
they make it easy to catch some kinds of mistakes that might otherwise
be missed).  But in this case the infrastructure for automating tests
doesn't yet exist, and would take some time to create.  I _am_ opposed
to making that task a higher priority than fixing urgent and
relatively simple bugs (like this one).

Comment 90

[Steven Michaud [:smichaud] (Retired)]

> (like this one)

(like this one now is)

Comment 91

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

We need this for major update, but won't block 3.0.1 for it. Please indeed expedite the review process, and even if what we do is add litmus tests which use the key event utility that's linked to in this bug, we should do that.

I'm going to require a good amount of bake time for this. All due respect, but these fixes (esp. without tests) have a nasty tendency to cause other regressions that we can't risk publishing on a branch release.

Comment 92

[Steven Michaud [:smichaud] (Retired)]

I've finished version 1.0 of a simple NPAPI plugin to test the
browser/plugin interface on OS X.  It displays the events that get
sent to the plugin, and also logs them (along with other information)
to stdout/stderr.  I've posted it at bug 441880.

Here's another tryserver build made with my rev7 patch (attachment
323438 [review]).  The previous one (from comment #79) is almost 30 days old,
and will get deleted soon.

https://build.mozilla.org/tryserver-builds/2008-06-25_12:[e-mail]-bugzilla357670-rev7/[e-mail]

Comment 93

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

Can we get this reviewed and in on mozilla-central?

Comment 94

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

Gen: can you get some people using the test build (see comment 92) and ensuring that this solves the IME/Flash problems without regressing anything else?

Comment 95

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 323438 [details] [diff] [review]
Patch rev7 (fix bug 434914)

Roc, just so that you're aware:

My patch is also available (in attachment 323440 [details]) as a patch against
the "original" (pre-backout) code.

I did more than was strictly necessary to fix 434914.  But all my
additional changes are (I think) very simple and straightforward, and
create no additional risk.  To put it another way, they should have
been there from the first :-)

Comment 96

[Gen Kanai [:gen]]

(In reply to comment #94)
> Gen: can you get some people using the test build (see comment 92) and ensuring
> that this solves the IME/Flash problems without regressing anything else?

Yep, we'll get the word out on this and try and collect feedback in this bug asap.

Comment 97

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 323438 [details] [diff] [review]
Patch rev7 (fix bug 434914)

But I recommend to the 3.0.1 drivers that we not land this patch until we have automated tests for this bug and bug 434914.

Comment 98

[Steven Michaud [:smichaud] (Retired)]

> But I recommend to the 3.0.1 drivers that we not land this patch
> until we have automated tests for this bug and bug 434914.

I don't think there's any way to get automated tests (_reliable_,
_tested_ automated tests) in that timeframe.

As I already mentioned above, the basic problem (for any kind of test)
is that we need to know what events arrive at the plugin, in which
order (and at what time).  This information would have to come from
the plugin (presumably a descendant of my testing plugin from comment
#92).  But we don't (yet) have any way to feed this information to one
of our automated test frameworks.

Comment 99

[Mike Shaver (:shaver emeritus)]

Automated doesn't mean that it has to be run by the tinderbox on day 0.  If you can click "test" and get back "pass" or "fail" for the set of things it covers, that's IMO automation enough to land.  If a human has to remember to run all of the dozens of tests by hand and interpret the results, we're in trouble.

Once we have some automated (though not automatic, if you see what I mean) test to work from, it'll be easier to figure out how to get it into a framework, and which will accomodate it better.

(I think we have test frameworks that read firefox's stderr and parse it, so just dumping the pass/fail stuff there would probably be an easy hack, but I'm not too concerned about that part.)

Comment 100

[Steven Michaud [:smichaud] (Retired)]

> If a human has to remember to run all of the dozens of tests by hand
> and interpret the results, we're in trouble.

The problem that remains here is very simple.  It doesn't need "dozens
of tests".  In fact it needs just one -- does the plugin receive a
mouse-down event when it has the keyboard focus and a key is pressed,
and does it receive a mouse-up when the key is released.

Yes, I think we need to evolve semi-automated and (eventually)
fully-automated tests of the browser/plugin interface.  But it's
pointless, and in fact deeply counter-productive, to try to do that
for _this_ bug, and before the Firefox 3.0.1 release.

Comment 101

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #100)

What's really needed here is "live", open-ended tests (particularly)
of a representative sample of Flash games.  Beyond the obvious (and
very simple) test that I mentioned in comment #100, I frankly don't
know what kinds of demands Flash games may place on this bug's code.

So the highest priority here is not automated tests (or rote tests of
any kind) -- it's tests that will answer the question I asked above
("what kinds of demands ...").

I have a better handle on ordinary text-input (in Flash), because I've
already done more testing of that.  But open-ended tests of this are
also needed -- particularly of the popular Japanese sites that not
having this bug fixed currently prevents the use of.

So Gen, you have your work cut out :-)

Comment 102

[Gen Kanai [:gen]]

(In reply to comment #101)
>
> So Gen, you have your work cut out :-)
> 

We'll do our best to get the word out asap. 

Comment 103

[Jon]

As per Gens request...

Just tested on Minefield 3.0.1 @ youtube
http://youtube.com/watch?v=2gxe7Jnxewo
Japanese comment in the video at 9 seconds (done via youtube's video-annotation Flash interface) worked fine.

Mac OSX 10.5.3

Comment 104

[matt romaine]

confirmed that it works on NicoNico Video

http://www.nicovideo.jp/watch/sm3811045
http://www.nicovideo.jp/watch/sm3813756

need to test UStream.tv where I also had this bug before, but don't have anyone with a live stream up at the moment.

Comment 105

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think that we need to test for confirming there are no regressions rather than IME works fine...

E.g., the key handling on other plug-ins work fine. Java, Quicktime, etc.

Comment 106

[dynamis (Tomoya ASAI)]

I also tested rev7 test build and works fine.
I can input Japanese with Kotoeri/Atok on Mac OS X 10.4/10.5.
Both key up/down events are send to flash and I can play flash games fine:
https://bugzilla.mozilla.org/show_bug.cgi?id=434914#c4
http://www.kongregate.com/games/arawkins/dolphin-olympics-2

Masayuki:
for other plug-ins, do you know any example for Java, Quicktime etc?

Comment 107

[Kazuho Oku]

(In reply to comment #92)

I could input Japanese however inline input was NOT possible (a separate window popped up for kana-kanji translation).

Tested on Mac OS X 10.4.11 (x86), Flash Player 9,0,124,0, at ustream.tv and nicovideo.jp.

Comment 108

[dynamis (Tomoya ASAI)]

Quicktime test results following file:
http://people.mozilla.com/~pkim/downloaddaymozilla.mov

I can pause/start with space bar and back/forward with left/right key.

note:
I cannot use space bar when I turn on IME (Kotoeri or Atok) but it's not problem of this patch nor firefox. It's the problem of Quicktime plug-in who don't treat space key as pause/start button when IME is ON.

(In reply to comment #107)
> I could input Japanese however inline input was NOT possible (a separate window
> popped up for kana-kanji translation).

Yes. That's same behavior as Safari, problem of Flash plug-in, not problem of browsers side. Even if we can make workaround for it, we should treat it as a separate bug I think.

Comment 109

[dynamis (Tomoya ASAI)]

Concerning text filed within Java Applet like:
http://chaichan.web.infoseek.co.jp/src/jlearn08.htm
We cannot use IME within the text filed. IME will be disabled when we focus on the text field but this is not the bug of the patch nor the firefox. It's problem (spec) of the Java Applet (or spec) and same as Safari/Opera behavior.

And some more test results I received from Japanese testers:
 - All shortcut keys worked fine for Quicktime except spacebar with IME on (as I wrote on comment #108, quicktime side problem).
 - No problem to play some Java Applet games like: 
   http://games.yahoo.co.jp/games/login.html?page=dc
 - Can input Japanese correctly into other flash text fields like:
   http://www.playercore.com/bugFiles/ime/imekrjp.swf

I will not write every test results but as far as Japanese community testers check, no problem found with the test build. 
Thanks.

Comment 110

[dynamis (Tomoya ASAI)]

There are some Java Applets with Japanese input text fields like:
http://www.sunflat.net/ja/
And we can input Japanese text into the text field of the Java Applet with the test build and no regression found.


[Note] Below report is NOT regression:
But I found the case in which I cannot input Japanese with the test build on OS X 10.5:
Step to Reproduce:
1. go: http://fl.sunflat.net/badminton/pc.php
2. Input some Japanese Name in the text filed (works fine)
3. Click「ゲームをプレイ」Button
4. just wait a few seconds for the gave over.
5. 3 menu items are shown and select「[5] 結果を見る」menu with "5" key or select「[*] 他のゲーム」with "*" key.
6. Back to pc.php with back button of the browser.
7. (without clicking other place) Click「ランキングに使う名前」text field
8. IME doesn't work on the text field :(
9. Click anywhere other than the text field.
10. Click text field again.
11. Now IME works and I can input Japanese text.

The problem (step 8) could only be reproduced only with my iMac (Mac OS X 10.5) so far. Not reproduced with my Mac Book Pro (Mac OS X 10.4). This may bug of:
 - just my iMac env
 - Java Applet itself
 - Firefox on Mac OS X 10.5
   # even in this case, not regression

Comment 111

[Steven Michaud [:smichaud] (Retired)]

Thanks for the tests!  I'm very grateful.

A)

The QuickTime problem reported in comment #108 also happens in Safari
(on both OS X 10.4.11 and 10.5.3).  As Tomoya says, this is
(presumably) a problem with Apple's QuickTime plugin.

B)

The jlearn Java applet problem reported in comment #109 also happens
in Safari, and only on OS X 10.5.X (not on 10.4.11).  As Tomoya
reported, the problem is that all non-Roman input-modes/keyboards are
disabled.  I'd guess this is a problem with the applet, and perhaps
also an Apple bug.

By the way, on the Mac Java applets don't get their events from the
browser (via the NPAPI) -- instead they get them directly from the OS.
So this patch is very unlikely to have any effect on Java or Java
applets.  (I'm the author of the Java Embedding Plugin that implements
Java in OS X distros of Mozilla.org browsers.)

C)

The problem reported in comment #110 happens in the Minefield rev7
build (from comment #92) but not in Safari (on both OS X 10.4.11 and
10.5.3).

The badminton game appears to be a Flash "movie", not a Java applet
(disabling the Flash plugin makes it stop displaying).

I can't tell whether the "Name for ranking" ランキングに使う名前 field
the Flash movie or in HTML code.  Can you tell me, Tomoya?

Comment 112

[Atsushi Sakai]

(In reply to comment #111)
> B)
> the problem is that all non-Roman input-modes/keyboards are
> disabled. 

This problem occurs with other Java applet, too.
For example, daifuku chat http://www.daifukuya.com/chat/applet.html

But we can change to Hiragana mode by pressing Ctrl+Shift+J,
unless Windows-like operation is enabled in Kotoeri preferences.
I think this is not related to our bug, though.

> I can't tell whether the "Name for ranking" ランキングに使う名前
> field
> the Flash movie or in HTML code.

"Name for ranking" ランキングに使う名前 is in Flash movie.
See http://fl.sunflat.net/pctitle.swf

Comment 113

[Steven Michaud [:smichaud] (Retired)]

Landed patch rev7 (attachment 323438 [details] [diff] [review]) on mozilla central.  Tomorrow's
mozilla-central nightly will have this patch (as will subsequent
mozilla-central nightlies).

I'm now supposed to mark this bug FIXED ... but I hesitate to do so
until the patch has also been landed on the 1.9.0 branch.  I'm afraid
doing so would confuse people reporting test results here.

(In reply to comment #112)

Thanks, Atsushi, for your info on the "Name for ranking" field.

I'm going to spend the rest of the day looking into the badminton game
problem, to see if there's something I can (and should) do about it
for the 3.0.1 release.

Comment 114

[Mike Shaver (:shaver emeritus)]

You should absolutely mark it fixed, IMO.  Bugs that are in consideration for the branch but not FIXED to indicate that they've stuck on trunk are painful and troubling for release drivers.

Comment 115

[Y.Okuno]

I confirmed the operation in the IRC client of the Java applet named QuickIRC.

MacOSX 10.5.3
AquaSKK 3.6 http://aquaskk.sourceforge.jp/
QuickIRC http://irc2.2ch.net/QuickIRC/qi-login.cgi

I was able to input the character without trouble. I was able also to switch the character input without trouble. 

Comment 116

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #114)

OK, marking this FIXED.

But this bug _isn't_ yet fixed for the next release (Firefox 3.0.1).
That will only happen when a patch lands on the 1.9.0 branch.

Comment 117

[Shaw Hosaka]

(In reply to comment #113)
> I'm going to spend the rest of the day looking into the badminton game
> problem, to see if there's something I can (and should) do about it
> for the 3.0.1 release.
Firefox2(on Mac OSX 10.5) happens to the badminton game problem. 

This badminton game problem doesn't occur in Safari because of the thing that the control rule of focus is different from Firefox.

Comment 118

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #117)

> Firefox2(on Mac OSX 10.5) happens to the badminton game problem.

Confirmed.  I see the same problem in FF2, on both OS X 10.4.11 and
10.5.3.  So this problem isn't a regression (from FF2).

> This badminton game problem doesn't occur in Safari because of the
> thing that the control rule of focus is different from Firefox.

I don't understand.  Please give more detail.

Comment 119

[Shaw Hosaka]

(In reply to comment #118)
> I don't understand.  Please give more detail.
This case is Firefox gives Flash a focus with priority, but Safari does not give Flash a focus.
To confirm this, the space key is pushed on the page. If the page doesn't scroll, focus is given to Flash. 

Comment 120

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #119)

In other words, in step 6 (and 7 and 8) of comment #110's STR, Firefox
is trying to focus the Flash movie (the one containing the "Name for
ranking" text-field).  But it somehow doesn't manage to do this right
(the Flash movie is somehow only partially focused).

But Safari, in step 6 of comment #110's STR, doesn't try to focus the
Flash movie containing the "Name for ranking" text-field.  The "Name
for ranking" field only gets focused (and focused correctly) when you
click on it.

The puzzle is that when the page containing the Flash movie loads for
the first time, Firefox _doesn't_ try to focus the Flash movie (and
the "Name for ranking" field works just fine, after you've clicked on
it).  Furthermore, the Flash movie doesn't get focused if you reload
the page at step 6 (and again the "Name for ranking" field works fine
when you click on it).

This may have something to do with Firefox's page caching.  Though the
behavior didn't change when I mirrored the badminton site and added a
"cache-control: no-store" header to pc.php.

In any case, I don't think the "badminton problem" should hold up
landing this bug's patch.

It isn't a regression (from FF2), and will take a while to figure out
properly.  Once I've made some headway I'll open a new bug on it.

Comment 121

[Steven Michaud [:smichaud] (Retired)]

A big thank-you to all the people who've tested my rev7 patch
tryserver build (from comment #92)!!  And thanks to Gen for calling
you in.

Things have gone very well.  No problems have been found related to my
patch.  And very few problems of any kind have been found.

I think we're almost done.  But I _would_ like to see a few more tests
of Flash games.

Thanks in advance!

Comment 122

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #121)

Well, if I want the job done I guess I've go to do it myself.

I found a bunch of free Flash games that use keyboard navigation, and
tested them out (using my tryserver build from comment #92).  I went
to two sites and tested four "action" games in each ("action" games
are more likely to use keyboard navigation).  All of these games'
navigation keys worked correctly.

It'd be nice if someone could confirm my results.  But in any case I
think I've done enough to show that keyboard navigation should work
correctly in Flash games.


AddictingGames (http://www.addictinggames.com/index.html)

DogFight: The Great War
http://www.addictinggames.com/dogfight.html

Rock & Roll Space Monkey
http://www.addictinggames.com/rockrollspacemonkey.html

Destroy the World
http://www.addictinggames.com/destroytheworld.html

Park My Car
http://www.addictinggames.com/parkmycar.html


UGotGames (http://www.ugotgames.com/)

Skywire
http://www.ugotgames.com/action/skywire.php

Shrubbery
http://www.ugotgames.com/action/shrubbery.php

Ignite People on Fire
http://www.ugotgames.com/action/ignite-people-on-fire.php

Rollercoaster Rush
http://www.ugotgames.com/action/rollercoaster-rush.php

Comment 123

[Marcia Knous [:marcia]]

Steven: As soon as I finish my assigned testing for 3.0.1, I will do some additional testing of flash games that use keyboard navigation.

Comment 124

[Steven Michaud [:smichaud] (Retired)]

Thank you, Marcia!!

Comment 125

[Marcia Knous [:marcia]]

Using Steven's Tryserver build in Comment 92, I tested the following games on 10.4.5:

http://www.2flashgames.com/fullscreen.php?id=882
http://www.ugotgames.com/action/rollercoaster-rush.php
http://www.ugoplayer.com/games/virtualkeyboard.html
http://www.actionflash.com/matrix_fighting.php
http://www.2flashgames.com/fullscreen.php?id=5997

In each case the keyboard navigational controls worked fine. I tried to test some of the same games as Steven did as well as try some that were hosted on different sites.  I will do some testing using 10.4.11 as well.

Comment 126

[Marcia Knous [:marcia]]

All of the games noted in Comment 125 also work using the tryserver build and Intel 10.4.11.

Comment 127

[Steven Michaud [:smichaud] (Retired)]

Marcia, thanks for your tests!

Are you planning to do any more?  (Not that I think any more are
needed, as far as I can tell.)

Comment 128

[Marcia Knous [:marcia]]

I wasn't planning on it, but the only other thing I could try is a few tests running on PPC mac.

(In reply to comment #127)
> Marcia, thanks for your tests!
> 
> Are you planning to do any more?  (Not that I think any more are
> needed, as far as I can tell.)
> 

Comment 129

[Steven Michaud [:smichaud] (Retired)]

Marcia, do whatever you think is needed.

A few PPC sanity checks might help, but there's no reason (that I'm aware of) to expect different behavior on PPC and Intel.

Comment 130

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 323438 [details] [diff] [review]
Patch rev7 (fix bug 434914)

This patch has now been tested very thoroughly.

The revised patch (rev7) has, if anything, been tested even more
thoroughly than the original patch, and has now been sitting on
mozilla-central for a bit over two weeks.  Only one (relatively minor)
problem was found (see comment #120), and that problem isn't a
regression (it also exists in FF2).

But even the original patch was tested quite thoroughly (notably by
Atsushi Sakai), and then rev6 sat on the trunk (what's now become the
1.9.0 branch) for more than a month before bug 434914 was opened and
the patch was backed out.  No other problems were reported during this
time.

Now that 3.0.1 is out, this patch should be landed on the 1.9.0
branch, to give it time to bake there.

Comment 131

[Samuel Sidler (old account; do not CC)]

Comment on attachment 323438 [details] [diff] [review]
Patch rev7 (fix bug 434914)

I agree we should get this in sooner rather than later so any potential regressions can be found. We especially need tests for this, at the very least a Litmus test or two.


Approved for 1.9.0.2. Please land in CVS asap so we can get baking. a=ss

Comment 132

[Steven Michaud [:smichaud] (Retired)]

Landed on the 1.9.0 branch:

Checking in widget/src/cocoa/nsAppShell.mm;
/cvsroot/mozilla/widget/src/cocoa/nsAppShell.mm,v  <--  nsAppShell.mm
new revision: 1.33; previous revision: 1.32
done
Checking in widget/src/cocoa/nsChildView.h;
/cvsroot/mozilla/widget/src/cocoa/nsChildView.h,v  <--  nsChildView.h
new revision: 1.95; previous revision: 1.94
done
Checking in widget/src/cocoa/nsChildView.mm;
/cvsroot/mozilla/widget/src/cocoa/nsChildView.mm,v  <--  nsChildView.mm
new revision: 1.361; previous revision: 1.360
done

Comment 133

[Shane Caraveo]

I've had to revert this patch as it broke key handling in Komodo (on 1.9 branch).  The reason is that no keypress event is sent to plugins, a change from before, and different than win/lin.  nsObjectFrame actually prevents the keypress from getting to the plugin on no-osx platforms, but we were able to capture the event prior to that.  Our keybinding system depends on having that event.  I also no longer receive text events for IME, which we intercepted to handle IME in scintilla.

Comment 134

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Excuse me, why Komodo needs to capture the keypress events before nsObjectFrame? When a plugin has focus, others should not process the key events. Plugins on MacOS X need only keydown/keyup events for text inputting. So, Gecko don't need to generate keypress and any IME events.

Comment 135

[Shane Caraveo]

When IME is utilized, I don't need the keypress event, but I do need the text event which used to get sent.  It is not sent with the changes in this bug.  Reverting that I do receive the IME text event.

Keypress events were always sent prior to this change, and are still sent on win/linux.  We have a keybinding system that is very dynamic and relies on the keypress events.  Again, reverting the patch gives me the keypress events.

Comment 136

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #133)

I agree with Masayuki -- you shouldn't be trying to get a keypress
event from the browser.

And I can't figure out either how you did this, or why you'd want to.
It can't (I don't think) be the timing -- keypress events are
generated at the same time as keydown events.  It might help if you
gave a much more detailed explanation of "why" and "how".

I notice that Komodo is neither open source nor free.  But ActiveState
has another program, Komodo Edit, that appears to be both.  Does
Komodo Edit have the same problem?  And if so, where should we look in
its source code to see what's going on?

(By the way, you can use my DebugEventsPlugin from bug 441880 that
neither FF2 nor FF3 send keypress events to plugins, even on OS X.)

Comment 137

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #135)

I frankly don't understand what you're saying.

Is there somewhere we can look in the Komodo Edit source code to see
what you're talking about?

Comment 138

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #135)
> When IME is utilized, I don't need the keypress event, but I do need the text
> event which used to get sent.  It is not sent with the changes in this bug. 
> Reverting that I do receive the IME text event.

Yes, you're right. keypress events and text events are not fired at same time. If in composing, text events are sent, otherwise, keypress events are sent. So, I meant in previous my comment that we don't send  these events on Mac when plugin has focus. Because plugin don't need these events and we cannot generate these events easily.

> Keypress events were always sent prior to this change, and are still sent on
> win/linux.  We have a keybinding system that is very dynamic and relies on the
> keypress events.  Again, reverting the patch gives me the keypress events.

Yeah, but that is a cause of this bug. We must not *eat* the IME processing by us. It should be handled in plugins.

Comment 139

[Steven Michaud [:smichaud] (Retired)]

Another question for you, Shane:

Does Komodo (and/or Komodo Edit) support CJK IME with WebKit browsers?
If so you might be able to use the same strategy with Firefox 3.

As of my patch(es) for this bug, Firefox 3 now uses the same methods
to support IME in plugins as does the WebKit.

(I'd still like answers to my questions above about Komodo Edit.  It'd
be _really_ nice to see your side of this problem in source code.)

Comment 140

[Shane Caraveo]

(In reply to comment #136)
> (In reply to comment #133)
> 
> I agree with Masayuki -- you shouldn't be trying to get a keypress
> event from the browser.

Not sure if you are using "browser" generically here, but this is a XUL application, not in a browser tag.

> And I can't figure out either how you did this, or why you'd want to.
> It can't (I don't think) be the timing -- keypress events are
> generated at the same time as keydown events.  

Prior to patch:

- keypress event was sent when plugin had focus and user was NOT in IME
- you could "capture" the event in DOM (XUL) before nsObjectFrame received it
- nsObjectFrame would prevent the event from reaching the plugin, and cancel bubbling.
- When in IME, a text event would be sent via DOM.  We could use that from the plugin to handle IME events easily.
- This is how it has worked for many years

After patch:

- Keypress events are no longer sent at all, even when NOT using IME
- IME text events are no longer sent

When we were using 1.8 branch, we patched nsObjectFrame to allow the bubbling of the events, but that is not necessary, and doesn't help at this point anyway since the keypress event is no longer sent.  I'm removing those patches (in our move to 1.9) and making small changes so we don't have to rely on the bubbling of the events.

> It might help if you
> gave a much more detailed explanation of "why" and "how".

Keypress events, unlike keydown/up have the char code correctly converted, and happen only once during a key sequence (ie. no keypress when simply hitting the modifier keys).  We built a customizable keybinding system in JS that allows our users to create their own keybindings, including fairly good emacs and vi emulation.  

We catch the keypress and see if it is a keybinding then run the command.  We need to do this even if the plugin has focus.

If it is not a keybinding, then we have a xbl widget (that wraps the plugin), which also catches the keypress event prior to the plugin, and does additional work (such as autocompletion, macro magic, etc) before using the plugin scriptable interfaces to pass in what we want to send to the editor.  

If we had to rely on keydown/up we'd have to figure out how to do the correct conversions.

> I notice that Komodo is neither open source nor free.  But ActiveState
> has another program, Komodo Edit, that appears to be both.  Does
> Komodo Edit have the same problem?  And if so, where should we look in
> its source code to see what's going on?

Komodo IDE is the commercial product that is on top of Komodo Edit.  This particular issue and the code around it are in Edit.  I can give you urls to the code, but see below.

> (By the way, you can use my DebugEventsPlugin from bug 441880 that
> neither FF2 nor FF3 send keypress events to plugins, even on OS X.)

I've seen it.  That plugin will not show the difference, because nsObjectFrame blocks keypress events from getting to the plugin. 

You can create a test case by creating a xbl component that has an html:embed as the content.  That embed loads your test plugin.  In the xbl component, add "keyup", "keydown", "keypress" and "text" handlers for both the capture and bubbling phase (dump a message from them).  Load the xbl component in chrome (not a browser tag).  Click on the plugin to focus it, then type.

Comment 141

[Shane Caraveo]

(In reply to comment #139)
> Another question for you, Shane:
> 
> Does Komodo (and/or Komodo Edit) support CJK IME with WebKit browsers?
> If so you might be able to use the same strategy with Firefox 3.

Komodo is a XUL application, it wouldn't work in WebKit.

> As of my patch(es) for this bug, Firefox 3 now uses the same methods
> to support IME in plugins as does the WebKit.

But it also changes behavior of the events that we've relied on for years.

> (I'd still like answers to my questions above about Komodo Edit.  It'd
> be _really_ nice to see your side of this problem in source code.)

My description of the test case in my prior response would give a better (and much easier) view of what is going on.  Try it with and without the patches.

Comment 142

[Shane Caraveo]

(In reply to comment #138)
> (In reply to comment #135)
> > When IME is utilized, I don't need the keypress event, but I do need the text
> > event which used to get sent.  It is not sent with the changes in this bug. 
> > Reverting that I do receive the IME text event.
> 
> Yes, you're right. keypress events and text events are not fired at same time.

That's not what I was saying.  I don't need them to be fired at the same time.  I need to receive dom events for keypress (when not using IME) and text events (when using IME).  nsObjectFrame does the appropriate thing and prevents them from reaching the plugin, so it doesn't affect plugins, but it does affect XUL applications like Komodo.

> If in composing, text events are sent, otherwise, keypress events are sent. 

I spent all weekend debugging this and understanding the code in nsChildView.mm, they are not sent at all, at least not via DOM as they are when I remove the patch.

> So,
> I meant in previous my comment that we don't send  these events on Mac when
> plugin has focus. Because plugin don't need these events and we cannot generate
> these events easily.

Thus my problem.  I don't care if the plugin itself gets them, I do care that a DOM event is not generated.  It breaks my application which has depended on that behavior for years.

> > Keypress events were always sent prior to this change, and are still sent on
> > win/linux.  We have a keybinding system that is very dynamic and relies on the
> > keypress events.  Again, reverting the patch gives me the keypress events.
> 
> Yeah, but that is a cause of this bug. We must not *eat* the IME processing by
> us. It should be handled in plugins.

I could live with having to change how IME is handled on osx for our plugin, but the lack of keypress when *not* using IME kills our application.  Scintilla (our editing component) does have IME support (provided by Adobe some time ago).  I enabled that over the weekend, and it is not invoked.  That's  due to not receiving carbon events when scintilla is embeded in the plugin.  I'm not certain how to get that working.  It seems like carbon events are not passed through to the plugin, but that could be something wrong in our plugin adapter (SciMoz).

Comment 143

[Shane Caraveo]

Some code pointers for Komodo Edit.

SciMoz is our plugin adaptor that embeds Scintilla, the editing component.

http://svn.openkomodo.com/openkomodo/browse/openkomodo/trunk/src/SciMoz

Scintilla OSX layer:

http://svn.openkomodo.com/openkomodo/browse/openkomodo/trunk/contrib/scintilla/macosx

Scintilla/SciMoz XBL wrapper:

http://svn.openkomodo.com/openkomodo/view/openkomodo/trunk/src/chrome/komodo/content/bindings/scintilla.p.xml

Comment 144

[Steven Michaud [:smichaud] (Retired)]

Thanks, Shane, for the detailed information you've provided.

It will take me a while to work through it, and come up with
suggestions.

I'd like to have something by the end of this week ... but don't be
surprised if I take longer.

Comment 145

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #142)
> > > Keypress events were always sent prior to this change, and are still sent on
> > > win/linux.  We have a keybinding system that is very dynamic and relies on the
> > > keypress events.  Again, reverting the patch gives me the keypress events.
> > 
> > Yeah, but that is a cause of this bug. We must not *eat* the IME processing by
> > us. It should be handled in plugins.
> 
> I could live with having to change how IME is handled on osx for our plugin,
> but the lack of keypress when *not* using IME kills our application.  Scintilla
> (our editing component) does have IME support (provided by Adobe some time
> ago).  I enabled that over the weekend, and it is not invoked.  That's  due to
> not receiving carbon events when scintilla is embeded in the plugin.  I'm not
> certain how to get that working.  It seems like carbon events are not passed
> through to the plugin, but that could be something wrong in our plugin adapter
> (SciMoz).

You should not think to separate keypress event and text event. *Both* events are fired in same point that is insertText of nsChildView.mm when gecko has focus.

insertText is called by [super interpretKeyEvents:].
[super interpretKeyEvents:] is called by keyDown.

Probably, [super interpretKeyEvents:] converts the key events to inputting string with low level APIs. However, now, we don't call [super interpretKeyEvents:] when plugin has focus. And we cannot call it under current design, probably.



I think that if Komodo is a XUL application, it must not handle key events and IME events itself. Because we cannot support IME on XUL's DOM event level. IME behavior (especially, the modern text inputting mechanism) is too complex, therefore, we support the text inputting features only on the editor of Gecko. I don't have a plan that we support them in DOM events level. So, XUL application developers should not implement the editors themselves, they should use the editor of Gecko.

Otherwise, if Komodo is a plug-in application, it must handle native key events itself from keydown events on Mac, like other plug-ins. We might support both design plug-ins, however, I have no idea for it.

And note that fx3 is the first release of Cocoa widget based Fx. And this bug's change could not be landed to Fx3.0.0, unfortunately. 

Comment 146

[Shane Caraveo]

(In reply to comment #145)

> 
> You should not think to separate keypress event and text event. *Both* events
> are fired in same point that is insertText of nsChildView.mm when gecko has
> focus.

They are no longer fired if a plugin has focus, and that is the problem.

> I think that if Komodo is a XUL application, it must not handle key events and
> IME events itself. 

In XBL we capture the "text" event and pass it through to the xpcom layer in our plugin, which then uses the nsIPrivateTextEvent (and related) interfaces to deal with IME.  The end result is something that looks/behaves almost exactly like IME in any text widget in mozilla.  

see the text event handler, which passes the text event to the plugin in http://svn.openkomodo.com/openkomodo/view/openkomodo/trunk/src/chrome/komodo/content/bindings/scintilla.p.xml

handleTextEvent is implemented in:

http://svn.openkomodo.com/openkomodo/view/openkomodo/trunk/src/SciMoz/nsSciMoz.cxx

Just search for "SciMoz::HandleTextEvent".  I have to fix our setting of textEventReply->mCursorPosition, but otherwise this method makes IME work for us in a platform independent way.

> Because we cannot support IME on XUL's DOM event level. 

From my point of view, you have been supporting IME via DOM with the text event, and we have used it quite successfully for a long time.

> So, XUL
> application developers should not implement the editors themselves, they should
> use the editor of Gecko.

The editors in Gecko are woefully inadequate for our needs.

Comment 147

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #146)
> (In reply to comment #145)
> > I think that if Komodo is a XUL application, it must not handle key events and
> > IME events itself. 
> 
> In XBL we capture the "text" event and pass it through to the xpcom layer in
> our plugin, which then uses the nsIPrivateTextEvent (and related) interfaces to
> deal with IME.  The end result is something that looks/behaves almost exactly
> like IME in any text widget in mozilla.  

Note that IMEs are not simple text input assistant. E.g., they can request to get the content text. Such method is used for reconversion (bug 348341). And the around text of insertion point is very good reference for IMEs which guess the user wanted result and they can sort the candidate list of the conversion. It's used by modern Japanese IMEs, actually.

Note that such complex behavior needs very many events. And they can be also used by other applications, e.g., screen reader. And if we implement the events to DOM events, we need very many hard work. Of course, ideally, we should implement the DOM events. However, we are not in such step, unfortunately.

For we fire both keypress event and text event, we need to emulate [super interpretKeyEvents:] ourselves. But I have no idea for it. Steven may know this way.

> > So, XUL
> > application developers should not implement the editors themselves, they should
> > use the editor of Gecko.
> 
> The editors in Gecko are woefully inadequate for our needs.

Yeah, I can guess it easily. But I believe that nsIEditor inherited class implementation may be good choice for XUL developers if they want to create a customized editor. 

The plug-in mechanism (NPAPI) has many serious bugs of IME :-(

Comment 148

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #147)

> For we fire both keypress event and text event, we need to emulate
> [super interpretKeyEvents:] ourselves. But I have no idea for
> it. Steven may know this way.

I don't know.  Though I'll be looking into this possibility when I go
over the information Shane's given me (above) -- later this week or
possibly next week.

Here's something you should be aware of, Shane (if you're not already)
-- the new Cocoa NPAPI event model (bug 435041) will support IME in
plugins (those that use the NPAPI) more-or-less automatically.

Especially if Komodo and Komodo Edit don't support IME on Firefox 2
(something I'm not yet clear on), you may wish to postpone support for
them on Firefox 3.X until Firefox 3.X supports the Cocoa NPAPI event
model.

Comment 149

[Shane Caraveo]

(In reply to comment #148)
> (In reply to comment #147)
> 
> > For we fire both keypress event and text event, we need to emulate
> > [super interpretKeyEvents:] ourselves. But I have no idea for
> > it. Steven may know this way.
> 
> I don't know.  Though I'll be looking into this possibility when I go
> over the information Shane's given me (above) -- later this week or
> possibly next week.
> 
> Here's something you should be aware of, Shane (if you're not already)
> -- the new Cocoa NPAPI event model (bug 435041) will support IME in
> plugins (those that use the NPAPI) more-or-less automatically.

This is what I kind of assumed this patch did, but as I couldn't make it work I had to back out to keep what we already had working.

> Especially if Komodo and Komodo Edit don't support IME on Firefox 2
> (something I'm not yet clear on), you may wish to postpone support for
> them on Firefox 3.X until Firefox 3.X supports the Cocoa NPAPI event
> model.

Komodo has had IME working through the text event since 3.5 or 4.0 (cant remember right now). k3.x was on gecko 1.7, k4.x has been on gecko 1.8.  k5.0 is going to be on 1.9.0.  We'll likely move up to 1.9.1 on some point release.

Unfortunately, Scintilla is a mix of HIView/Carbon and QD, I'm not sure when I'll be able to get around to doing a Cocoa port.

Comment 150

[Steven Michaud [:smichaud] (Retired)]

>> Here's something you should be aware of, Shane (if you're not
>> already) -- the new Cocoa NPAPI event model (bug 435041) will
>> support IME in > plugins (those that use the NPAPI) more-or-less
>> automatically.
>
> This is what I kind of assumed this patch did, ...

It does (support IME more-or-less automatically).  But only for
straight NPAPI plugins like the Flash plugin, and then only using an
input window (not the in-place IME that's available in the browser
proper).

Comment 151

[Steven Michaud [:smichaud] (Retired)]

To clarify, the new Cocoa NPAPI event model will (or should) support
in-place IME (more-or-less automatically) in a plugin that supports
this event model.

No released versions (of any browser) currently do (support the Cocoa
NPAPI event model) ... though you can hack something like it into the
current Safari release, if you build current WebKit code from scratch
and use scripts that are provided there.

Comment 152

[Steven Michaud [:smichaud] (Retired)]

Shane, just to make sure I've understood correctly:

I've pulled the current Open Komodo source code and downloaded today's
nightly.  Running the program (and looking at the build instructions
for the source code), it appears that it bundles its own copy of
Firefox, patched with a number of Komodo-specific patches.  (It also
appears that the bundled Firefox version is 2.0.0.X.)

So (I assume) the fact that Firefox 3.0.2 will contain this patch (for
bug 357670) won't break users' copies of Komodo.  And so it's not a
showstopper that this patch breaks (or appears to break) Komodo.  If
you had to, you could back out this patch (as one of your
Komodo-specific changes) when you move to bundling Firefox 3.0.X (aka
1.9.0-branch Firefox).

Yes, I can understand why you'd prefer not to do this.  But it appears
to be a workable option, if we're not able to find some other way to
fix (or work around) this problem.

Comment 153

[Shane Caraveo]

(In reply to comment #152)
> Shane, just to make sure I've understood correctly:
> 
> I've pulled the current Open Komodo source code and downloaded today's
> nightly.  Running the program (and looking at the build instructions
> for the source code), it appears that it bundles its own copy of
> Firefox, patched with a number of Komodo-specific patches.  (It also
> appears that the bundled Firefox version is 2.0.0.X.)

We need to delete those nightly builds, they are relics.  Komodo Edit from downloads.activestate.com is what you would get.  4.x versions do run on the 1.8 branch, essentially a patched xulrunner with our own executable.  5.0a1 is the first build on 1.9 branch.  We do bundle our own version.

> So (I assume) the fact that Firefox 3.0.2 will contain this patch (for
> bug 357670) won't break users' copies of Komodo.  And so it's not a
> showstopper that this patch breaks (or appears to break) Komodo.  If
> you had to, you could back out this patch (as one of your
> Komodo-specific changes) when you move to bundling Firefox 3.0.X (aka
> 1.9.0-branch Firefox).

> Yes, I can understand why you'd prefer not to do this.  But it appears
> to be a workable option, if we're not able to find some other way to
> fix (or work around) this problem.

Yes, we can always patch, but that is not desirable.  Every patch involves maintenance over time, and has other consequences, such as never being able to be part of a linux distribution, never being able to use other plugins, always having to spend time tracking down new bugs that appear to see if they are a consequence of the patches.

Comment 154

[Steven Michaud [:smichaud] (Retired)]

> Komodo Edit from downloads.activestate.com is what you should get.

Thanks.  I see the download for the 5.0 alpha1 there.

Also, now that I've finished building and running the nightly I pulled from openkomodo.com (using svn co http://svn.openkomodo.com/repos/openkomodo/trunk openkomodo), I see (from the page's appearance) that it uses 1.9 branch Firefox code.

Comment 155

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #140)

>> (By the way, you can use my DebugEventsPlugin from bug 441880 that
>> neither FF2 nor FF3 send keypress events to plugins, even on OS X.)
>
> I've seen it.  That plugin will not show the difference, because
> nsObjectFrame blocks keypress events from getting to the plugin.
>
> You can create a test case by creating a xbl component that has an
> html:embed as the content.  That embed loads your test plugin.  In
> the xbl component, add "keyup", "keydown", "keypress" and "text"
> handlers for both the capture and bubbling phase (dump a message
> from them).  Load the xbl component in chrome (not a browser tag).
> Click on the plugin to focus it, then type.

I'm going to need help doing this.

Please tell me where I can find code template(s), or attach some to
this bug (or perhaps to bug 441880).

Comment 156

[Shane Caraveo]

(In reply to comment #155)
> Please tell me where I can find code template(s), or attach some to
> this bug (or perhaps to bug 441880).

see bug 441880 for a xpi I quickly threw together.  That should give you an idea of the test case I'm looking at.  It probably isn't entirely working yet, but should be 90% there.

Comment 157

[tmkk]

This bug again on 4.0 beta, regardless of OOPP enabled/disabled.

Comment 158

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #157)

What you're seeing is probably bug 571537.

Please download the latest version of the Flash plugin (from Adobe, not Apple) and try again.

Comment 159

[tmkk]

In 4.0b3 that bug is fixed and ASCII texts can be typed, but IM is not enabled. I'm using the latest flash plugin (10.1.82.76) on 10.6.4. Java plugin is also problematic.

Comment 160

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #159)

Sigh, you're right.

But what you report is a different bug, because it has a different
cause -- bug 587667.

> Java plugin is also problematic.

Trunk builds (on OS X) now use Apple's port of Sun's Java Plugin2
(their out-of-process NPAPI Java plugin).  The meta-bug for problems
with this plugin is bug 554103.  If you find a new Java Plugin2 bug,
please open a Bugzilla bug report and add it to bug 544103's "depends
on" list.

Comment 165

[Malik Rumsey]

(In reply to Steven Michaud [:smichaud] (Retired) from comment #25)

Turns out I had to make fairly substantial changes to get my patch to
work in both Minefield and Camino. (I now attach my Carbon text-event
handler to the event dispatcher target, and I've stopped storing any
information in the WindowDelegate class (which isn't available to
Camino and other embedders).)

I tested the patch on the three URLs mentioned elsewhere in this bug,
in both OS X 10.5.2 and OS X 10.4.11 (in both Minefield and Camino):

http://www.playercore.com/bugFiles/ime/imekrjp.swf
http://examples.adobe.com/flex3/consulting/styleexplorer/Flex3StyleExplorer.
html
https://dirtyharuto.com/ja/video/8440757374680341352/
http://www.playercore.com/bugfiles/146162/AddReturnHTML.html

Once again, I'd really like to have more testcases, using different
plugins.

Mostly everything seems to work correctly. But there are still some
oddball problems ... which don't seem important compared to the
problems I've solved, but which are still annoying:

Remaining problems (that I'm aware of):

1. When (only in Minefield) the focus is in a Flash plugin, you can't
   select either of the Thai keyboards (both are greyed out in the
   Flags menu).

   I'm not sure why this happens, but I suspect that it's because,
   when the Flash plugin (running in Minefield) creates its own TSM
   document (using NewTSMDocument()), its single supported interface
   is of type kTextServiceDocumentInterfaceType (I found this out in
   gdb).

   This problem doesn't happen in Camino ... because (oddly) the Flash
   plugin never calls NewTSMDocument() when it's running in Camino.
   So I have the browser create a TSM document for use by plugins that
   (like Camino) don't create their own (IME won't work without an
   active, non-Cocoa TSM document). When I create this TSM document I
   make it support a single interface of type
   kUnicodeDocumentInterfaceType. I'm pretty sure this is why Camino
   doesn't have the disabled-Thai-keyboards problem.

2. If (only in Minefield, only on OS X 10.4.11) I load one of the
   Flash/Flex examples (listed above) while a particular keyboard is
   active in the browser, I can't change to a different keyboard for
   input while that plugin is focused. In order to change the
   keyboard (or IME style) in a plugin, I have to change it in the
   browser and then reload the plugin.

   Once again I suspect the Flash plugin is somehow at fault ... but I
   don't know how. I really want to be able to test in some other
   kind of plugin (like presumably the Adobe plugin) that doesn't do
   it's own Cocoa-based IME (as the Java Embedding Plugin does).

Here's a tryserver build made with this patch:

https://build.mozilla.org/tryserver-builds/2008-04-09_16:02-smichaud@pobox.
com-bugzilla357670/[e-mail]

I don't have no idea for this bug.

We don't send any native events to plugin at IME events. Because there are no native events for IME(TSM) on cocoa. When key is downed, the keyDown event is only occurred. We call interpretKeyEvents of super class in keyDown. Then interpretKeyEvents calls NSTextInput methods for IME. So, if we need to send native carbon event (EventRecord) to plugins, we need to generate it ourselves. However, there are no documents for TSM events of EventRecord. We need to know what events are needed by plugins.