[viewpoint] keyboard events aren't passed to windowless plugin correctly

VERIFIED FIXED in mozilla0.9.9



16 years ago
14 years ago


(Reporter: Ari Berger, Assigned: Peter Lubczynski)


Windows NT

Firefox Tracking Flags

(Not tracked)




(1 attachment, 2 obsolete attachments)



16 years ago
From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.01; Windows NT 5.0)
BuildID:    0.9.4 branch

1) I only get keyboard events when I the plugin's page is in a frame.
View the following URL:
We get the keyboard events (NPP_HandleEvent is called)
View the following:
(exact same content, just not viewed through index, which creates the frame)
No keyboard events.

2) Note that either way, you will not see the effects of typing.
The second problem is that we are getting the wrong info in wparam and lparam.
Here is MSDN:
wParam - Specifies the virtual-key code of the nonsystem key. 
lParam - Specifies the repeat count, scan code, extended-key flag, context 
code, previous key-state flag, and transition-state flag, as shown in the 
following table. 
Here is your code in nsWindow.cpp line 2420:
  pluginEvent.wParam = 0;
  pluginEvent.wParam |= (event.isShift) ? MK_SHIFT : 0;
  pluginEvent.wParam |= (event.isControl) ? MK_CONTROL : 0;
  pluginEvent.lParam = aVirtualCharCode;
Looks to me like lParam is geting virtual char info, and wParam is getting 
shift and control info. First of all, I think they are flipped.  Second, is the 
shift and control info in the same form that WM_KEYDOWN or WM_KEYUP expects?

Reproducible: Always

Peter's comment from 116392:
I'll take a look at the frameset problem, but I don't see it in my simple
testcase. My key events get to my windowless plugin without a frameset after I
give the plugin focus by clicking in it's area. Perhaps that's the problem?
I assume it is not just framesets that would allow the events to get through- 
probably other things would do- maybe div tags?
You should be able to reproduce the problem at the URL that I gave.
I did make sure to give the window the focus first.

I will try your patch for flipping lParam and wParam.

Comment 1

16 years ago
This bug is spun off from bug 116392. See this patch for problem #2:

Problem #1 is a little bit more complicated. It appears that it's the margins on
the BODY tag which are causing the problem. Add this attribute to the BODY tag
and the problem goes away:
style="margin: 0em"

That's not a very good workaround. So, here's what I see in the debugger:

The event is dispatched through the views but never makes it to the plugin's
view because the key event's coordinates (0,0) aren't "inside" the CanvasFrame's
view when it has a margin:

Since the canvas frame's default origin is offset by 8 pixels but the key event
is at zero, the event is given the document to handle instead of the child view.

I'm not quite sure what can be done here. Anyone have any ideas? Do I need to
"target" the key event so its coordinates are that of my frame's? It looks like
there was some code commented out that did something like that a long time ago:
Assignee: av → peterl
Ever confirmed: true
Keywords: edt0.9.4

Comment 2

16 years ago
Actually, looking at |nsViewManager::DispatchEvent| where it looks like we
target the event based on widget may be a problem with windowless plugins which
have no widget. :( The event is correctly sent to the document since that's the
widget that got the key event. Strange that it works when we have no margins.

Since my plugin has no widget, how can I get my key event? Could I simply attach
my nsIDOMKeyListener on the document instead of the plugin in the case of
windowless? Or is there a special kind of widget that can bet used with this
kind of plugin?
While I was working on the view manager event code recently I wondered why
non-mouse events are dispatched specifically to (0,0), but I was afraid to
change it :-). Presumably only mouse events should go through the
coordinate-based dispatch path, but who knows what might break if we changed it.

I think this is a focus problem. I don't really know how DOM/layout focus works
but I think somehow you should be telling nsEventStateManager to make your
plugin the focus so it will forward key events to you.

nsIViewManager::GrabKeyEvents lets you tell the view manager to send events
directly to a particular view, but no-one uses that at the moment so if I were
you I'd only use it as a last resort.

Comment 4

16 years ago
Created attachment 67090 [details] [diff] [review]
possible patch? (listening to key events on the documents)

Here's a possible patch that seems to work for me in the debugger in getting
windowless plugins key events. It attaches an extra DOM key event listener to
the document. The only draw back is that the plugin now gets ALL the key events
of the document which I'm not quite sure is correct. This patch also flips
wparam and propigates lparam in widget code for key events. Ari, can you try
this out and let me know if I'm close?

I also noticed that my plugin isn't getting a DOM focus/blur event when there
is a margin on the document. This could be causing the problem with key events,
the content never gets the focus. But it still may be the same underlying
problem that windowless plugins don't have widgets. I'll continue to

Comment 5

16 years ago
What is tthe status
Plugins certainly should not just steal all key events for the document. They
should participate in focus management with the rest of the document. Think
about what's going to happen if there are multiple plugins in a page, or they're
in a page with form controls.

Have you looked at the way form controls get focused? Maybe you can copy that.
Many form controls don't have widgets.

Comment 7

16 years ago
Created attachment 67106 [details] [diff] [review]
have plugin set focus

Yeah, scratch that last patch. Ari, try this one, it's a little better.

So I compared form controls and noticed it was really a focus problem. I guess
since we don't have a widget, we need to set focus ourselves. In any case,
doing this at MouseDown time seems to fire all the listeners.
Attachment #67090 - Attachment is obsolete: true

Comment 8

16 years ago
Ari, Does the patch work? And is this a stopper?

Comment 9

16 years ago
The patch fixes this test case.  Tomorrow we will test it in a few more 
situations.  But it looks good.

Comment 10

16 years ago
Created attachment 67479 [details] [diff] [review]
new patch

Ari let me know that I he was getting double key events in some cases. This was
because I made keypress events go through as well as keydown in my patch in bug
116392. NPAPI doesn't say anything about keypress, so they should just be
Attachment #67106 - Attachment is obsolete: true

Comment 11

16 years ago
please review
Keywords: patch, review
Priority: -- → P1
Target Milestone: --- → mozilla0.9.9

Comment 13

16 years ago
Comment on attachment 67479 [details] [diff] [review]
new patch

Does this patch prevent JS type event handlers from getting keyboard eve ts if
a plugin consumes the events?
Attachment #67479 - Flags: superreview+

Comment 14

16 years ago
Plusing for 0.9.4
Keywords: edt0.9.4 → edt0.9.4+

Comment 15

16 years ago
Yes, the 'onkeypress' handler appears to still fire correctly despite me
consuming it.

Patch in trunk.
Last Resolved: 16 years ago
Keywords: review
Resolution: --- → FIXED

Comment 16

16 years ago
Has this landed in 094 yet? Please add "fixed0.9.4" KW as expected.

Comment 17

16 years ago
Keywords: fixed0.9.4


16 years ago
Keywords: verified0.9.4

Comment 18

16 years ago
testcase works ok on 0206 branch build. 

Comment 19

16 years ago
hey, the layout on both the testcases is messed up  on the trunk (0306) . can
anyone confirm ? However, the keybd events work fine ( textfields seen
partially..butI can type in it) in both tests...

Comment 20

16 years ago
I have been waiting for bug 118721 to get fixed before I start trying to figure 
out how we work against the trunk- that causes layout problems on most of our 

Comment 21

16 years ago
keyboard events do work great on this test now. verif with 0812 trunk.


14 years ago
Keywords: fixed0.9.4
You need to log in before you can comment on or make changes to this bug.