159016 - [OSX] Inline IME does not work in composer and text area when the Flash 6 plugin is running

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P3)

Description

[Teruko Kobayashi]

Inline IME does not work in composer and text area on Mac OS X 10.1.5 Japanese.

Steps of reproduce
1. Launch Netscape
2. Open the composer
3. Change the Japanese Kotoeri and change to Hiragana mode
4. Start typing Japanese 

Actual result
Whatever you type Japanese Hiragana, it will display in the different dialog.

Expected result
When you type Hiragana in Composer, the hiragana will be displayed whenver you type.

The similar thing happens in browser.
1. Go to http://www.yahoo.co.jp
2. Type Japanese characters in Text submit area

Hiragana characters will be displayed in the different dialog.

Tested 7-23 Mac OS X branch build. We could reproduce this on Mac OSX 10.1.5.
I cannot reproduce this on Mac OSX 10.1.3 and 10.1.4.

Comment 1

[Teruko Kobayashi]

This is very bad.  I nominate this to nsbeta1.

Comment 2

[Teruko Kobayashi]

I tested this with 70PR1. This is not a regression from 70PR1.
However, this is a regression from Netscape 6.23.

Comment 3

[Rui Xu]

Here is a related bug http://bugscape.mcom.com/show_bug.cgi?id=15298 with more 
test results. They might be caused by the same reason.

Comment 4

[Frank Tang]

I think this is a ADT1 bug. Show stoper. also topembed.

Comment 5

[Frank Tang]

this is my higest priority today.

Comment 6

[Teruko Kobayashi]

Frank and I found out this is not a regression from 6.23.  This is related to
Shockwave Flash veriosn.

Comment 7

[Frank Tang]

ok. I got more info.
I am sure it is realted to flash.
1. if there are no html page which contains flash display on the screen
(regardless it is in the same page or not), then we don't have this problem on
html nor composer
2. if there are a flash display in some window, composer or that page will have
problem . once you close that window which have flash, then the composer won't
have the problem. I guess flash suck some IME or keyboard event from us once it
display on the screen. The problem happen on teruko and my machine which the
about plugins say the Flash version is 6.0r29. In the same machine we can
reproduce this on n6.2.3. We cannot reproduce this on roy's machine and his
flash is 6.0r58

I need some help for who know Mac plugin to help me

Comment 8

[Frank Tang]

does the netscape build include the flash or it come from some where else?
Is flash include with MacOS X 10.1.5 ?

Comment 9

[Frank Tang]

Attachment: sample flash file

Comment 10

[Frank Tang]

Attachment: test cases demotracte the problem.

Comment 11

[Frank Tang]

Attachment: same as above with embed comment out

if the last page is not display on the screen and no other page have flash
display on the screen, you won's see the problem in this page.

Comment 12

[saari (gone)]

CC'ing Peter L. and Brian Nesse

Comment 13

[Peter Lubczynski]

How do I see the failure using that last testcase on OSX? Regular typing works
in my branch build. How do I test Japanese? I'm using 10.1.5 w/ Flash 6.0r40

Since 6.2.3 seems to show the same problem and you can't reproduce in 10.1.3 or
10.1.4, this sounds like a new problem in 10.1.5 rather than a regression.

Where did you get Flash 6.0 r58 ??? I thought the LATEST version JUST released
is 6.0r40 which ALSO has upgrades to support scriptability. Does the latest
version of Flash solve this problem? If it does, maybe we should ship that
version which fixes scripting on OSX?

Comment 14

[Frank Tang]

sorry typo in my previous comment:
>We cannot reproduce this on roy's machine and his flash is 6.0r58
shuld be
>We cannot reproduce this on roy's machine and his flash is 5.0r58 (it is 5r58
not 6r58) 
arun said on MacOS X , netscape does not ship with flash plugin. The flash
plugin is installed in a system global place by macromedia and probably also
apple. Probably Apple's MacOS X 10.1.5 update install the Flash 6r29 which have
this problem
I update the Flash from macroedia download site directly  the Flash 6 r40. It
also have the problem. 

Comment 15

[Paul Wyskoczka]

Lowering to adt2 on behalf of the adt.

Comment 16

[Frank Tang]

Attachment: Best patch for now untill the plugin vendor fix their plugin.

this will have bad side effect which impact plugin or embedding application
which call UseInputWindow with tsmdoc and true. Maybe we should do NOTHING in
Gecko and let macromedia release a new Flash which take out that UseInputWindow
with NULL call (I don't mind they call with their own tsmdoc, but call it will
NULL is not their job)

Comment 17

[Frank Tang]

don't like that patch
here is some option to solve this issue
1. do nothing in gecko and ask macromedia to change the Flash
2. take my hacky patch- this is NOT good for embedding client which need to call
UseInputWindow(myTSMDocument, true);
3. put UseInputWindow(mTSMDocument,false) after NewTSMDocument and register a
observer. on mac after we initial the pluginl, call the observers. The observers
call UseInputWindow(mTSMDocument,false) . This is different from my previous
patch because it only call UseInputWindow with the TSMDocument we care. The
patch will be about 50-100 lines of code. I will try it today. However, it won't
solve the problem for embedding client because if some widget expect to work as
inline, Flash will turn the inline off after it initial. And if some widget
expect to work as user input window, flash will turn the inline ON after it
shutdown. 

I really think the right way to solve it is to solve it from the root. Which
mean ask for a new Flash 

Comment 18

[Frank Tang]

no luck to work around this issue inside Gecko. 
I add some code to call UseInputWindow(myTSMDoc, false) but it does not fix the
problem for us.

Comment 19

[Frank Tang]

Attachment: patch does not work

this is the patch that try to work around the problem by callign UseInputMethod
back to false after every time a plugin is initialized. However, it does not
work. not sure why

Comment 20

[Frank Tang]

this is really caused by a bug in FLASH instead of buggy code in mozilla.
Waiting for Macromedia to fix it in a new version of FLASH

Comment 21

[Yuying Long]

This happens on Mac OS 10.2 Juguar also.

Comment 22

[saari (gone)]

Is there any update from Macromedia on this bug?

Comment 23

[Arun Ranganathan]

Update from Macromedia:

"I am working with the team in order to get you an update in terms of the 2 hot
items."

Comment 24

[Frank Tang]

since this is really a macromedia issue and aruner is working on the
communication with them, reassign this bug to aruner. Aruner, please let me know
if they still need any info from me.
Chris Saari- please talk to aruner directly about this issue. Macromedia have
already recognize all the inforamation we send to them is valid and know
whatever we know. Since aruner is already tracking the progress with them and
they don't need any more technical now, I will leave it to aruner to do the rest.
 

Comment 25

[Frank Tang]

we might find a work around if Macromedia can tell me all the places that they
call ::UseInputWindow , they mention NPP_Initialization and NPP_Shutdown but I
believe that they call it in more places than that two. 

Comment 26

[Frank Tang]

I suggest we take
http://bugzilla.mozilla.org/attachment.cgi?id=92795&action=view to fix the problem 

Comment 27

[Peter Lubczynski]

Comment on attachment 92991 [details] [diff] [review]
patch does not work

Should we maybe do this after calls to SetWindow?

Comment 28

[Peter Lubczynski]

Comment on attachment 92795 [details] [diff] [review]
Best patch for now untill the plugin vendor fix their plugin.

oops, last comment was for  wrong attachment. Should we maybe do
::UseInputWindow after each call to SetWindow or do we have to do it once per
plugin instance or just once after general plugin initilization like is done
here? Is there any harm in making this call multiple times?

Comment 29

[Frank Tang]

according to the problem plugin vendor, they call the ::UseInputWindow(NULL,
TRUE) in the NPP_Initialize, that is why I put that call here. (AFter the
NPP_Initialize)
A better fix will be detect which plugin is called in the NPP_Initialize and
only call the ::UseInputWindow(NULL, FALSE) after the known problem plugin.
PeterL, what do you think ?

Comment 30

[Frank Tang]

according to the embedding customer I work with, he said
>We have one instance where it is called with true.
>
>In that instance, the TSMDocumentID we have to associate is nil. Looking >at
the code this is a failsafe. Normally, the code will have a class >member that
indicates the document ID, and will call UseInputWindow with >this doc ID and
false for the floating window parameter.

Therefore, I think it is ok to use
http://bugzilla.mozilla.org/attachment.cgi?id=92795&action=view
untill the plugin vendor to fix it. 

Comment 31

[Frank Tang]

Comment on attachment 92991 [details] [diff] [review]
patch does not work

this does not work anyway

Comment 32

[Peter Lubczynski]

Attachment: same hack, but just for Flash 6 on Carbon

Frank, what do you think of this patch to just do this hack for Flash 6 on
Carbon? Hopefully Flash >6 and other plugins will not have this problem. :)

Comment 33

[Frank Tang]

Attachment: additional code to bullet proff

PeterL's last patch won't compiled because of lacking a n in the strcasecmp
function I also add additional code in the widget so in case some other code
call the UseInputWindow(null, true), we turn a particular tsmdocument to false

Comment 34

[Peter Lubczynski]

Comment on attachment 96941 [details] [diff] [review]
additional code to bullet proff

r=peterl

Comment 35

[Simon Fraser [no longer active]]

+#if defined(XP_MAC) && TARGET_CARBON

Do you want it to apply to MachO builds too (photon/chimera/...)?

Comment 36

[Frank Tang]

>Do you want it to apply to MachO builds too (photon/chimera/...)?


we should, chimera have the same problem. Try the test cases above. 

Comment 37

[Frank Tang]

sfraser said
XP_MAC is "undefined" in chimera
TARGET_CARBON is "defined" in chimera

Comment 38

[Simon Fraser [no longer active]]

CFM builds (MachV):
XP_MAC is 1
XP_MACOSX is 0
TARGET_CARBON is 1 for OS X build, 0 for OS 9 buld

MachO builds (Chimera etc)
XP_MAC is 0
XP_MACOSX is 1
TARGET_CARBON is 1

Comment 39

[Frank Tang]

sfraser: does this mean we should change
+#if defined(XP_MAC) && TARGET_CARBON

to
+#if TARGET_CARBON
and then it will work ?

Comment 40

[Simon Fraser [no longer active]]

I talked to peterl on the phone about this.

I'd prefer:

#if defined(XP_MAC) || defined(XP_MACOSX)
#if TARGET_CARBON
...
#endif
#endif

Comment 41

[Peter Lubczynski]

Comment on attachment 96941 [details] [diff] [review]
additional code to bullet proff

I'm afraid with this patch it may make it difficult for Macromedia to fix this
in the future. I think we've got to be even more specific to the version of
Flash.

Comment 42

[Peter Lubczynski]

Attachment: new patch with OSX defines and limited Flash 6 versions

Here's a new patch that includes the MACOSX defines and also limits this hack
to Flash 6 versions equal or older than r47.

Comment 43

[Simon Fraser [no longer active]]

+    if (pluginTag->mDescription &&
+        !PL_strncasecmp(pluginTag->mDescription, "Shockwave Flash 6.0", 19) &&
+        pluginTag->mDescription[21] <= '4')

This makes us very sensitive to the format of their "description" string. What
if they change that at some point? Is there any way to get a numeric version
number from the plugin?

Comment 44

[Frank Tang]

Comment on attachment 98603 [details] [diff] [review]
new patch with OSX defines and limited Flash 6 versions

should 
#include <TextServices.h>
inside 
 #if defined(XP_MAC) && TARGET_CARBON
?
or
inside 
 #if defined(XP_MAC) && TARGET_CARBON

change that then we shoudl carry a r=

Comment 45

[Frank Tang]

Comment on attachment 98603 [details] [diff] [review]
new patch with OSX defines and limited Flash 6 versions

take out the r=, 
what will happen to r5, r6 , r7, r8, r9, r50 ?
I think these release above should also apply the hack, right ?

Comment 46

[Frank Tang]

Comment on attachment 98603 [details] [diff] [review]
new patch with OSX defines and limited Flash 6 versions

what will happen if they have r100?

Comment 47

[Peter Lubczynski]

Macromedia says this will be fixed in a future version. I'm working on the fix
so taking bug back --->peterl

Comment 48

[Peter Lubczynski]

Attachment: patch v.2

Comment 49

[Simon Fraser [no longer active]]

-#if defined(XP_MAC) && TARGET_CARBON
+#if defined(XP_MAC) || defined(XP_MACOSX)
+#if TARGET_CARBON
 #include "nsIClassicPluginFactory.h"
+#include <TextServices.h>
 #endif

Aren't you missing an #endif here?

Comment 50

[Peter Lubczynski]

There already were two #endifs. I'm also removing the duplicate #include:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp#177
177 #if defined(XP_MAC) && TARGET_CARBON
178 #include "nsIClassicPluginFactory.h"
179 #endif
180 
181 #if defined(XP_MAC) && TARGET_CARBON
182 #include "nsIClassicPluginFactory.h"
183 #endif

Comment 51

[Frank Tang]

Comment on attachment 98815 [details] [diff] [review]
patch v.2

r=ftang

Comment 52

[Frank Tang]

sfraser: can you sr= it ?

Comment 53

[Simon Fraser [no longer active]]

Comment on attachment 98815 [details] [diff] [review]
patch v.2

sr=sfraser

Comment 54

[Peter Lubczynski]

Patch in trunk, marking FIXED:

/cvsroot/mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp,v
new revision: 1.430; previous revision: 1.429

/cvsroot/mozilla/widget/src/mac/nsMacEventHandler.cpp,v
new revision: 1.150; previous revision: 1.149

Comment 55

[Jaime Rodriguez, Jr.]

teruko: can you pls verify this as fixed in tomorrow's trunk builds? thanks!

Comment 56

[Jaime Rodriguez, Jr.]

This is blocking bug 150046 [META] Mac Embedding Blockers. Hence, I am gonna
mark this bug as Topembed+.

Comment 57

[Peter Lubczynski]

This turned the Mach-O tinderbox red last night. I backed out the Mach-O changes
because I wasn't sure how to resolve this error:
/usr/bin/ld: nsPluginHostImpl.o illegal reference to symbol: _UseInputWindow
defined in indirectly referenced dynamic library
/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox
make[4]: *** [libgkplugin.dylib] Error 1

I've got a Mach-O build now and could try a suggested fix otherwise I'll open a
new bug.

Comment 58

[Peter Lubczynski]

Attachment: makefile changes for Mach-O

I got some help from cls and these makefile changes are needed to build this
patch on Mach-O.

Comment 59

[Peter Lubczynski]

Attachment: real makefile changes (ignore last)

Comment 60

[Simon Fraser [no longer active]]

Comment on attachment 99107 [details] [diff] [review]
real makefile changes (ignore last)

sr=sfraser

Comment 61

[Brian Nesse (gone)]

Comment on attachment 99107 [details] [diff] [review]
real makefile changes (ignore last)

r=bnesse.

Comment 62

[Teruko Kobayashi]

I tested this in 2002-09-13-09 Netscape and PPEmbed Trunk build on Mac OS X
10.1.3, 10.1.5, and 10.2.

I did following tests.

Netscape build

Case 1
Steps of reproduce
1. Launch Netscape
2. Go to http://www.macromedia.co.jp 
3. Open the composer
4. Change the Japanese Kotoeri and change to Hiragana mode
5. Start typing Japanese 

Actual result
When you type Hiragana in Composer, the hiragana will be displayed whenver you
type. Inline IME works fine.

Case 2
1. Launch Netscape
2. Go to http://www.macromedia.co.jp 
3. Open new Navigator window
4. Go to http://www.google.co.jp
4. In the text area, change the Japanese Kotoeri and change to Hiragana mode
5. Start typing Japanese 

Actual result
When you type Hiragana in Composer, the hiragana will be displayed whenver you
type. Inline IME works fine.

Case 3
1. Launch Netscape
2. Go to http://www.yahoo.co.jp
3. Make sure Flash is running
4. In text area, change the Japanese Kotoeri and change to Hiragana mode
5. Start typing Japanese

Actual result
When you type Hiragana in Composer, the hiragana will be displayed whenver you
type. Inline IME works fine.

PPEmbed build
I did the Case 2 and 3. Inline IME works fine.

I just realized the performance of IME would slow down if Flash was running
(especially in Mac OS 10.2).

I verified this as fixed in 2002-09-13-09 Netscape and PPEmbed Trunk build on
Mac OS X 10.1.3, 10.1.5, and 10.2.
 

Comment 63

[Teruko Kobayashi]

marking verified.

Comment 64

[Jaime Rodriguez, Jr.]

edt1.0.2+ (per verbal from saari) approval for landing on the 1.0 branch,
pending Drivers' approval. Pls land time asap, the replace "mozilla1.0.2+" with
"fixed1.0.2". thanks!

Comment 65

[Peter Lubczynski]

Comment on attachment 99107 [details] [diff] [review]
real makefile changes (ignore last)

Talked with Simon,
+EXTRA_DSO_LIBS should be:
+EXTRA_DSO_LDOPTS
Mach-O changes are now in the trunk.

Comment 66

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 98815 [details] [diff] [review]
patch v.2

a=[e-mail] for 1.0 branch.  Please change mozilla1.0.2+ to fixed1.0.2
when checked in

Comment 67

[Peter Lubczynski]

patch in branch

Comment 68

[Jaime Rodriguez, Jr.]

teruko: can you pls verify this as fixed on the 1.0 branch, then replace
"fixed1.0.2" with "verified1.0.2". thanks!

Comment 69

[Teruko Kobayashi]

I verified this in 2002-09-19-05 Netscape and PPEmbed 1.0.2 branch builds on
MacOS X 10.1.3, 10.1.5, and 10.2.