Closed Bug 534027 Opened 11 years ago Closed 10 years ago

input event coords incorrect for oop windowless plugins

Categories

(Core :: Plug-ins, defect)

x86
Windows Vista
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(1 file, 1 obsolete file)

Because we translate the child's surface to a new origin, mouse event coords were off. This patch uses the origin information from the wm_windowposchanged event to correctly translate mouse events.
Attachment #416959 - Flags: review?(jmuizelaar)
Assignee: nobody → jmathies
Attached patch event updates patch v.2 (obsolete) — Splinter Review
A fix for a last minute change that caused a compile error - 'pRect' should be 'aRect' in UpdatePaintClipRect.
Comment on attachment 416962 [details] [diff] [review]
event updates patch v.2

(In reply to comment #1)
> Created an attachment (id=416962) [details]
> event updates patch v.2
> 
> A fix for a last minute change that caused a compile error - 'pRect' should be
> 'aRect' in UpdatePaintClipRect.

Oops, my queue got messed up. This should have been in bug 531860. I'll touch this up after reviews. :)
Attachment #416962 - Attachment is obsolete: true
It would be nice if the cosmetic changes were a separate patch from the functional ones.
Attachment #416959 - Flags: review?(jmuizelaar) → review+
Comment on attachment 416959 [details] [diff] [review]
event updates patch v.1

>diff --git a/dom/plugins/PluginInstanceParent.cpp b/dom/plugins/PluginInstanceParent.cpp
>--- a/dom/plugins/PluginInstanceParent.cpp
>+++ b/dom/plugins/PluginInstanceParent.cpp
>+            case WM_MOUSEMOVE:
>+            case WM_RBUTTONDOWN:
>+            case WM_MBUTTONDOWN:
>+            case WM_LBUTTONDOWN:
>+            case WM_LBUTTONUP:
>+            case WM_MBUTTONUP:
>+            case WM_RBUTTONUP:
>+            case WM_LBUTTONDBLCLK:
>+            case WM_MBUTTONDBLCLK:
>+            case WM_RBUTTONDBLCLK:
>+            {
>+                nsPoint pt(GET_X_LPARAM(npremoteevent.event.lParam), GET_Y_LPARAM(npremoteevent.event.lParam));
>+                pt.MoveBy(-mPluginPosOrigin.x, -mPluginPosOrigin.y);
>+                npremoteevent.event.lParam = MAKELPARAM(pt.x, pt.y);

Add a comment about where we are transforming the point from and to.

>+                if (!CallNPP_HandleEvent(npremoteevent, &handled))
>+                    return 0;
>@@ -661,6 +698,40 @@ PluginInstanceParent::AnswerNPN_PopPopup
> 
> /* windowless drawing helpers */
> 
>+/*
>+ * Origin info:
>+ *
>+ * windowless, offscreen:
>+ *
>+ * WM_WINDOWPOSCHANGED: origin is relative to container 
>+ * setwindow: origin is 0,0
>+ * WM_PAINT: origin is 0,0
>+ *
>+ * windowless, native:
>+ *
>+ * WM_WINDOWPOSCHANGED: origin is relative to container 
>+ * setwindow: origin is relative to container
>+ * WM_PAINT: origin is relative to container
>+ *
>+ * PluginInstanceParent:
>+ *
>+ * painting: mPluginPort (nsIntRect, set in setwindow)
>+ * event translation: mPluginPosOrigin (nsIntPoint, set in winpos.)
>+ */
>+
>+void
>+PluginInstanceParent::SharedSurfaceBeforePosChanged(NPRemoteEvent& npremoteevent)

I don't really like this name but can't think of a great alternative. Can the name describe what it's doing or why instead of when it's called? I don't know...

>+{
>+    WINDOWPOS* winpos = (WINDOWPOS*)npremoteevent.event.lParam;
>+
>+    // save the origin, we'll use this to translate input coordinates
>+    mPluginPosOrigin.x = winpos->x;
>+    mPluginPosOrigin.y = winpos->y;
>+
>+    winpos->x  = 0;
>+    winpos->y  = 0;
>+}
>+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Is there a way to mochitest this, or is that not possible because this is mouse events only which we can't synthesize from a test harness effectively?
Flags: in-testsuite?
(In reply to comment #5)
> Is there a way to mochitest this, or is that not possible because this is mouse
> events only which we can't synthesize from a test harness effectively?

Confirming a mouse event at a particular place within a plugin window would require simulated mouse positioning and changes to the test plugin to expose the info. Not sure about the former but the latter is certainly possible.
Depends on: 539020
You need to log in before you can comment on or make changes to this bug.