Closed
Bug 90574
Opened 23 years ago
Closed 23 years ago
Windowless plugins do not receive mouse/keyboard events
Categories
(Core Graveyard :: Plug-ins, defect, P1)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.4
People
(Reporter: kevint, Assigned: peterlubczynski-bugs)
References
()
Details
(Keywords: crash, topcrash, topembed, Whiteboard: topembed+)
Attachments
(6 files)
110.16 KB,
application/octet-stream
|
Details | |
192.71 KB,
application/x-zip-compressed
|
Details | |
13.64 KB,
patch
|
Details | Diff | Splinter Review | |
13.51 KB,
patch
|
Details | Diff | Splinter Review | |
14.31 KB,
patch
|
Details | Diff | Splinter Review | |
13.86 KB,
patch
|
Details | Diff | Splinter Review |
From Bugzilla Helper: BuildID: 20010627 If I go to a page with an embedded plugin (this happens with our Viewpoint Media Player plugin; I haven't seen it with others yet), and click on the embed area, Mozilla generates an assert in nsHTMLExternalObjSH::GetPluginInstance(), in the following line: CallQueryInterface(frame, &objectFrame); NS_ENSURE_TRUE(objectFrame, NS_ERROR_UNEXPECTED); Apparently the nsIFrame is expected to have an nsIObjectFrame, but it doesn't. I have enclosed a version of the plugin that has most of the functionality stripped out of it to make it easier to find the problem. Reproducible: Always Steps to Reproduce: 1.Put the enclosed plugin in the plugins directory 2.Go to this URL: http://samples.viewpoint.com/jewelry/Razolith/index.html 3.The page will load up, but you won't see any viewpoint content - that's ok because i stripped most everything out of the plugin. Just click on the tan rectangle in the middle of the window. 4. Mozilla should bring up an assert
Reporter | ||
Comment 1•23 years ago
|
||
Reporter | ||
Updated•23 years ago
|
Reporter | ||
Comment 3•23 years ago
|
||
fixed the url
Assignee | ||
Comment 4•23 years ago
|
||
Kevin, I can't get the sample plugin you attached to work in either Nav 4 or Mozilla. I think Bugzilla ate it. Could you attach again, perhaps zipped? Also, what is your specific problem or bug? The ASSERT by itself I don't think is a bug. It may even be out of date. I think it was added with jst XPCDOM landing. ------ Actually, looking at the comment for the method, it looks like this helps javascript call into the embed tag. I'd be good to know if there is a failure here: 3838 // HTMLObject/EmbedElement helper 3839 3840 // This resolve hook makes embed.nsIFoo work as if 3841 // QueryInterface(Components.interfaces.nsIFoo) was called on the 3842 // plugin instance, the result of calling QI, assuming it's 3843 // successful, will be defined on the embed element as a nsIFoo 3844 // property.
The attached plugin is a zip file. Just rename it after 'Save Link As' to something.zip.
Assignee | ||
Comment 6•23 years ago
|
||
Ahh, thanks Andrei. It looks like it's an INLINE frame instead of an OBJECT one. I recall we do some kind of frame substitution in nsObjectFrame::Reflow() that may account for this. Strange, as looking around the stack, It seems to be a NS_MENU_EVENT_START event which triggers the Javascript to go down this path. Funny thing is that I didn't click, I just hovered over the plugin. Is this even a problem? How does this block viewpoint? The markup also looks interesting. 1) You have a |source=| instead of |src=| on the embed tag. I think plugin code specifically looks for |src=| but I'm not sure and haven't dug into this. 2) Bug 90152 may effect this as it looks like the same sort of like <object><embed></object>
Reporter | ||
Comment 7•23 years ago
|
||
To address a couple of questions: First, we use "source=" instead of "src=" because we don't want Netscape starting the download immediately; we want to start it ourselves later on. Since we don't have the 'src' tag, we rely on the MIME type tag to tell Netscape to activate us. Second, about Bug 91052, I don't think this is the same problem, since we don't have any problem with rendering (this sample plugin doesn't render since I took all that code out). Third, as Peter noticed, the assert fires when the cursor hovers over the plugin. The reason this is a blocker is that we never get any mouse click if you do click on the plugin. Maybe the assert has nothing to do with this fact, they just seemed tied together.
Assignee | ||
Comment 8•23 years ago
|
||
Kevin, Could you further expalin, in more detail, how this bug is blocking you, specially the part about not getting key events. I'm trying to find out the "root" bug here as an ASSERT is not a bug, but probably will help in tracking down the bug. Also, repharsing the summary to be more descriptive about problem in general will help us triage it better. Also, have you tried removing the ASSERT or trying different markup? First try a simple testcase, with only an EMBED tag. Our code specifically looks for an SRC attribute so that could effect it. Try it both ways and if you see anything, please attach the testcase.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 9•23 years ago
|
||
Reporter | ||
Comment 10•23 years ago
|
||
OK, I think I was incorrect when I began describing the problem. I don't know if the assert has anything to do with the problem, but the crux of the issue is that windowless plugins do not receive mouse or keyboard events. It's my understanding that windowless plugins are supposed to receive these events through HandleEvent() (if I'm wrong here, please let me know, but this is how we do it for NS 4.x). Attached is the windowless plugin that was provided by David Brittain for bug #44322. If you put a breakpoint in HandleEvent(), it won't get called when you click on the plugin. Project files, a test .htm file, and a compiled dll are included in the .zip file attached.
Summary: Assert when clicking on embedded plugin → Windowless plugins do not receive mouse/keyboard events
Assignee | ||
Comment 11•23 years ago
|
||
Ahh...well that's a whole lot better summary. And thinking about the problem, I think I know what's up. On Win32, all events are sent to the plugin through child windows. Since a windowless plugin does not have a child window, no events get sent. Windowless plugins probably need to hook into the DOMEvent stuff that Mac uses (as it doesn't have child windows either). The code also probably need to be tuned to work on Windows instead of Mac. As a hack, try to change the #ifndef code around nsPluginInstanceOwner::DispatchMouseEvent and DisptachKeyEvent in nsObjectFrame.cpp. See if your events get through then.
Reporter | ||
Comment 12•23 years ago
|
||
Changing the #ifdef in DispatchMouseEvent does improve things a little bit, but there are still some peculiarities: I don't get ButtonDown events, but I get ButtonUp events, and I only get a MouseMove event when the cursor enters the plugin area, but not as it moves around inside the area (meaning if I move the cursor over the plugin, I get one mouse-move event only unless I repeatably move the mouse outside the embed area and back in again).
Assignee | ||
Comment 13•23 years ago
|
||
Right, this code was written for the Mac, so it probably needs to be tweeked for windowless plugins.
Assignee: av → peterlubczynski
Priority: -- → P1
Target Milestone: --- → mozilla0.9.3
Assignee | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: mozilla0.9.3 → ---
Assignee | ||
Comment 14•23 years ago
|
||
Kevin, Now that the fires have died down a bit, I've taken a closer look at windowless plugin events: I think if you repeat the hack you did for DispatchMouseEvent in nsPluginInstanceOwner::MouseDown, you may get your MouseDown events to reach the plugin. Let me know if that works for you as probably the same needs to be done for key and focus events. I'll try to figure out why MouseMove only fires once. Perhaps there is a flag that must be set.
Assignee | ||
Comment 15•23 years ago
|
||
Assignee | ||
Comment 16•23 years ago
|
||
Kevin, PLEASE try my patch and let me know if it makes things better.
Keywords: patch
Assignee | ||
Comment 17•23 years ago
|
||
Comment 18•23 years ago
|
||
Peter, you removed #ifndef XP_WIN. Should not we still ifdef it out for regular plugins on Windows and do this for windowless plugins only?
Assignee | ||
Comment 19•23 years ago
|
||
I'm still trying to find out the correct thing to do here, but I think you may be right. As with the case with keyevents, we crash in Win32.
Assignee | ||
Comment 20•23 years ago
|
||
Well, bug 93056 talks about SetWindow having incorrect coordinates while scolling for windowless plugins. That may be kind of difficult to fix as I recall it was hard to fix on the Mac.
Comment 21•23 years ago
|
||
is this still something we might want to fix on the 0.9.2 branch for the embedding customer in the next couple of weeks?
Comment 22•23 years ago
|
||
This is still something we ought to fix, and I'd like to lobby for a topembed+ . Kevin Tieskoetter, please jump in and comment here about how/why this is a blocker for you.
Assignee | ||
Comment 23•23 years ago
|
||
PDT Status: Waiting for Kevin Tieskoetter to review and test attachment 44056 [details] [diff] [review].
Reporter | ||
Comment 24•23 years ago
|
||
I've been trying out the fix that Peter provided, but I'm getting a very strange problem. It appears that, after adding the patch, my plugin is no longer getting any arguments (all the extra arguments in the object embed). Is there any way this patch could have caused this, or am I going completely mad? In tracing back into the Mozilla code, it seems now that in nsPluginInstancePeerImpl::GetAttributes(), when mOwner is queried for nsIPluginTagInfo, it returns NULL, so no attributes are attainable. I tried out the code just before applying the patch, and everything seemed to work OK... On the bright side, my plugin does seem to be getting mouse events now... :-)
Assignee | ||
Comment 25•23 years ago
|
||
Oh...I'm an idiot.....I forgot the nsIPluginTagInfo interface in my NS_IMPL_ISUPPORTS macro. It should read: +NS_IMPL_ISUPPORTS10(nsPluginInstanceOwner, + nsIPluginInstanceOwner, + nsIPluginTagInfo, + nsIPluginTagInfo2, + nsIJVMPluginTagInfo, + nsIEventListener, + nsITimerCallback, + nsIDOMMouseListener, + nsIDOMMouseMotionListener, + nsIDOMKeyListener, + nsIDOMFocusListener) Kevin, if you make that change, does it work for you then? Does that completely fix this bug for you or are there other issues? If you are satisfied, I'll make a patch for approvals and reviess. I still need to ensure we only do this in the windowless or Mac cases.
Whiteboard: [fix-in-hand]
Assignee | ||
Comment 26•23 years ago
|
||
Comment 27•23 years ago
|
||
I like the last one. r=av
Reporter | ||
Comment 28•23 years ago
|
||
Cool - the fix seems to work fine now. However, it then uncovered another bug that I didn't notice before, in that the window coordinates passed to NPP_SetWindow() are incorrect. I logged that as Bug #93500. Thanks for helping me out on this! :-)
Reporter | ||
Comment 29•23 years ago
|
||
To answer Arun's question, this bug is a blocker for us, since it prevents us (or any windowless plugin) from receiving any mouse or keyboard events.
Assignee | ||
Comment 30•23 years ago
|
||
Kevin, I think your new bug may be a dup of bug 93056. Can you try THIS patch (by David Brittain) and see if it fixes the coordinate problem: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=44370 If you are satisfied with this patch, I will get a super review, approvals and check it in.
Assignee | ||
Comment 31•23 years ago
|
||
Comment 32•23 years ago
|
||
sr=waterson
Assignee | ||
Comment 33•23 years ago
|
||
Fix on trunk, do I need a plus to check in on the branch?
Whiteboard: [fix-in-hand] → [fixed-on-trunk]
Comment 34•23 years ago
|
||
ok, check in on the branch. thanks
Assignee | ||
Comment 35•23 years ago
|
||
Chofmann, something got screwed up in bugzilla. This bug auto-morphed! attempting to change it back. (I assume the + was correct)
Summary: Crash in Trunk & N610 [@ 0x00000000 - PresShell::~PresShell] → Windowless plugins do not receive mouse/keyboard events
Assignee | ||
Updated•23 years ago
|
Assignee | ||
Comment 36•23 years ago
|
||
Checked into the MOZILLA_0_9_2_BRANCH, marking FIXED. Could someone see about getting a '+' for bug 93056?
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 38•23 years ago
|
||
verif that no assertions come up on clicking/typing inside the windowless plugin. verif on build 1120 trunk win.
Status: RESOLVED → VERIFIED
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•