310174 - Windowless plug-ins can't receive the focus events

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Masatoshi Kimura [:emk]]

Steps to reproduce:
1. Load the testcase.
2. Click on the Flash text input field.
3. Click on the HTML textarea.
Actual result:
Two carets are displayed.
Expected result:
Only one caret should be displayed.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Testcase

Comment 2

[Masatoshi Kimura [:emk]]

Attachment: Patch rv1.0

Comment 3

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

I think roc is better reviewer. Because he reviewed bug 306209.

Comment 4

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

-                    pluginEvent.event = 0x0047;
+                    pluginEvent.event = WM_WINDOWPOSCHANGED;

-  pluginEvent.event = 0x000F; //!!! This is bad, but is it better to include
<windows.h> for WM_PAINT only?
+  pluginEvent.event = WM_PAINT;

Is <windows.h> included now?

Are you sure you don't want to set wParam for these events?

Comment 5

[Masatoshi Kimura [:emk]]

> Is <windows.h> included now?
<winuser.h> is included instead, and it defines the WM_* constants.
> Are you sure you don't want to set wParam for these events?
MozClassic did the same thing.
http://lxr.mozilla.org/classic/source/cmd/winfe/cxwin.cpp#1296
Therefore all Nav4x windowless plug-ins will be able to handle this situation
(wParam = 0).

Comment 6

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

checed-in.

Comment 7

[Masatoshi Kimura [:emk]]

Oops. This patch didn't work on optimized build.
Reopening.

Comment 8

[Masatoshi Kimura [:emk]]

Attachment: A trivial patch resolving a scope problem

Comment 9

[Masatoshi Kimura [:emk]]

Attachment: Testcase

Hmm, too many events seems to be fired.
Load the testcase and click on the flash textbox.
Actual:
onfocus->onblur->onfocus->onblur...
Expected:
onfocus only once.

Comment 10

[Masatoshi Kimura [:emk]]

Attachment: New patch resolving the comment #9 problem

SetFocus hack in nsPluginInstanceOwner::MouseDown seems to be no longer
required.
This patch also fix the bug 310626 comment 20 problem.

Comment 11

[Masatoshi Kimura [:emk]]

Comment on attachment 198371 [details] [diff] [review]
New patch resolving the comment #9 problem

This patch was wrong.

Comment 12

[Masatoshi Kimura [:emk]]

Flash Player calls ::SetFocus(0) in the mouseDown handler by some unknown reason.

Comment 13

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

Does it mean Flash Player's issue?

Comment 14

[Masatoshi Kimura [:emk]]

Maybe.

Comment 15

[Masatoshi Kimura [:emk]]

http://lena.oersted.co.jp/~emk/2005/07/15/index_ns2.html
Load the above URL. When you are using
latest nightly: You can switch the tab without problems, but testcase in comment
#1 does not work.
latest debug build: You cannot switch the tab by click. Testcase in comment #1
works.
latest build with attachment 198371 [details] [diff] [review]: You can switch the tab only once. Testcase
in comment #1 works.

Comment 16

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

msintov:

See comment 12. We think that this bug is Flash Player's issue. Please check it.

Comment 17

[Masatoshi Kimura [:emk]]

I filed bug 311622 for Flash Player problem.

Comment 18

[Masatoshi Kimura [:emk]]

Attachment: Suppressing blur in MouseDown to workaround an issue about windowless Flash

Yes, this is ugly, but we have no choice until bug 311622 is fixed.

Comment 19

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

This doesn't seem like a serious enough bug to get a fix into Firefox 1.5 at
this stage.

Comment 20

[Masatoshi Kimura [:emk]]

I know.
I only want this patch to land on trunk.
This patch is not necessary for brach since Patch rv1.0 did not also land on branch.

Comment 21

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

I think we should not use this patch. This is not critical problem. Therefore,
Macromedia should fix bug 311622.

Comment 22

[Masatoshi Kimura [:emk]]

(In reply to comment #21)
> I think we should not use this patch. This is not critical problem. Therefore,
> Macromedia should fix bug 311622.
If we don't proceed, we should back out the attachment 197550 [details] [diff] [review]. It is not only
useless on release build, but also logically invalid (dangling pointer). If we
only fix it (by applying 198346), we can no longer click Flash movie on debug
build if wmode=opaque/transparent. I bet it will cause many many bug reports.
Hence I think it is serious enough to workaround.

Comment 23

[Masatoshi Kimura [:emk]]

> we can no longer click Flash movie on debug build if wmode=opaque/transparent.
we can no longer click Flash movie on *nightly* if wmode=opaque/transparent.

Comment 24

[Masatoshi Kimura [:emk]]

Comment on attachment 198346 [details] [diff] [review]
A trivial patch resolving a scope problem

Per Bugzilla-jp discussion, We (Masayuki and I) decided we do not want to take
a workaround for Flash.
Please review this patch.

Comment 25

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

checked-in to Trunk.

Comment 26

[Stephen Donner [:stephend] Not actively reading bugmail]

I think (but am not sure) that this caused bug 313230...