539565 - Flash plugin within a css transformed div does not handle mouse position properly

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Luc Devallonné]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_2; en-US) AppleWebKit/532.5 (KHTML, like Gecko) Chrome/4.0.249.30 Safari/532.5
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2) Gecko/20100105 Firefox/3.6

It seems that the mouse position send to the Flash plugin does not take into account the css transformed.

Reproducible: Always

Steps to Reproduce:
1.go to http://uniboard-misc.s3.amazonaws.com/plugin_transform.html
2.start movie
3.try to change movie position by clicking on the slider => does not work

Actual Results:  
The movie does not change position

Expected Results:  
The movie change position

If you click around 2/3 of the height of the movie, you can grab the position slider.

looks like the css scale transform of 1.5 is not applied to the mouse coordinates sent to the Flash plugin

Comment 1

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2pre) Gecko/20100113 Namoroka/3.6pre

Maybe related to Bug 538989. 
It is indeed not easy to grab the slider with the mouse.

Comment 2

[Luc Devallonné]

Bug 538989 is related to a windowed plugin, but in this case it is in windowsless mode

The content is properly display, just the mouse event are not transformed back to the plugin native coordinates

Comment 3

[Marcus Doemling]

I have observed this bug for a long time since at least Firefox 2.0 and frankly I'm amazed it's still there in 3.6 (only on the Mac).

Here are some other observations I can add. It seems that FF can get into a state in which this misalignment of detected and real mouse position occurs. When in this state the misalignment occurs, however when FF is not in this state there is no misalignment. One place where I observe this A LOT is hulu.com with their video player.

Another significant observation is I have found a way to put FF back from the "misaligning state" into the working state. This is done by resizing FF. When I observe this misalignment I resize FF by moving the bottom right corner just a bit. This causes the entire window to jump! This is important. I think it is jumping by the amount of the misalignment between detected and real mouse position. Once in the working state, resizing no longer causes the window to jump and there is no more misalignment, i.e. flash works as designed.

The fact that the window jumps leads me to believe this is really a FF issue, not a flash issue. I know there's always the suspicion that it might be the others, i.e. flashs, fault.

I feel this bug is so significant that it makes me look for alternatives. This deserves some attention!

Comment 4

[Victor]

For windows, FF already have a workaround; it translates events to flash as is but it informs flash about its' actual size.

For Mac, we can recompute events like this:

--- old/nsObjectFrame.cpp   2010-02-09 19:55:29.664159200 +0200
+++ new/nsObjectFrame.cpp   2010-02-18 19:35:33.314640800 +0200
@@ -484,10 +484,16 @@
     return "";
   }
 
+  // VK: just servant function in one line
+  PRBool IsShockwaveFlash()
+  {
+    return MatchPluginName("Shockwave Flash");
+  }
+
   PRBool SendNativeEvents()
   {
 #ifdef XP_WIN
-    return MatchPluginName("Shockwave Flash");
+    return IsShockwaveFlash();
 #else
     return PR_FALSE;
 #endif
@@ -4437,6 +4453,22 @@
         ::DeactivateTSMDocument(::TSMGetActiveDocument());
       }
 
+      // VK: adjust mouse pointer position for flash plug-in if transformed 
+        if (anEvent.eventStructType == NS_MOUSE_EVENT && IsShockwaveFlash()) { 
+           nsPoint pt = nsLayoutUtils::GetEventCoordinatesRelativeTo(&anEvent, mOwner)
+             - mOwner->GetUsedBorderAndPadding().TopLeft();
+           nsPresContext* presContext = mOwner->PresContext();
+           nsIntPoint ptPx(presContext->AppUnitsToDevPixels(pt.x),
+                        presContext->AppUnitsToDevPixels(pt.y));
+           EventRecord* er = (EventRecord*)anEvent.nativeMsg;
+           nsIntPoint scw = mWidget->WidgetToScreenOffset();
+           Point carbonPt = { ptPx.y + scw.y, ptPx.x + scw.x };
+           if(er) {
+             if (er->where.h != carbonPt.h || er->where.v != carbonPt.v) 
+               er->where = carbonPt;
+           }
+         }
+
       PRBool eventHandled = PR_FALSE;
       WindowRef window = FixUpPluginWindow(ePluginPaintEnable);
       if (window) {

Comment 5

[Victor]

Attachment: Recompute mouse pointer coords to current transform for flash plug-in, MacOS X only

Comment 6

[Mats Palmgren (inactive)]

Comment on attachment 427610 [details] [diff] [review]
Recompute mouse pointer coords to current transform for flash plug-in, MacOS X only

Your calculation of the coordinate looks good, but there are
a couple of things I'd like to change.  First, I don't see
any reason to only do this for Flash; second, we already have
essentially this code on lines 4417 -- 4422:
http://mxr.mozilla.org/mozilla1.9.2/source/layout/generic/nsObjectFrame.cpp#4408
I think it would be better to fix that code and
just calculate the coordinate once.

Comment 7

[Mats Palmgren (inactive)]

Attachment: Victor's patch with my suggested changes [for 1.9.2]

Victor, this is what I suggested above, does this look
right to you?

Comment 8

[Victor]

Sorry for long delay; yes, this way is much better. Thank  you very much.

Comment 9

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

Comment on attachment 428854 [details] [diff] [review]
Victor's patch with my suggested changes [for 1.9.2]

We should add a test for this to the Mac test plugin.

Comment 10

[Mats Palmgren (inactive)]

Attachment: Additional fix for testplugin on OSX [for 1.9.2]

Indeed.  After writing said test it revealed a bug in the
testplugin on OSX.  This patch just copies what is trunk
is already doing.
http://mxr.mozilla.org/mozilla-central/source/modules/plugin/test/testplugin/nptest_macosx.mm#334

Comment 11

[Mats Palmgren (inactive)]

Attachment: OSX fix for trunk

This is roughly what roc already reviewed above.  Can you check
that I got those NP_NO_CARBON/NPEventModelCarbon bits right please?

Comment 12

[Mats Palmgren (inactive)]

Attachment: mochitest.diff

Test mouse click coordinate with a few combinations of fullZoom
and transform:scale.  Sorry for slicing up the tests with
setTimeout(..., 0) but I couldn't make it pass reliably otherwise.
(something about zooming is async?)
Suggestions for improvements appreciated.

Comment 13

[Mats Palmgren (inactive)]

Haven't tested much on Windows yet, but the mochitest failed on m-c TryServer.
All tests seems to work on Linux (m-c and 1.9.2).

Comment 14

[Mats Palmgren (inactive)]

The test actually fails also on OSX with the attached fixes:
  fullZoom=2 MozTransform='' - got 271, expected 272
  fullZoom=2 MozTransform='scale(0.5)' - got 542, expected 544
which looks like a result of pixel rounding.
I think we should go ahead with the patches as is (unless someone
see something obviously wrong with them) and I'll make the test
tolerate the deviance above.  We can fix the pixel rounding later
in a separate bug.

Comment 15

[Mats Palmgren (inactive)]

Attachment: Testcase

Comment 16

[Mats Palmgren (inactive)]

Attachment: mochitest v2

Comment 17

[Mats Palmgren (inactive)]

On Windows trunk, the mouse event coordinate is correct but Flash
ignores the event if it's outside the non-transformed boundaries.
(STR: load the attached Testcase, Zoom-, set X-origin=1500px)
It works fine for the x-test plugin, and I debugged it for Flash
down to the point where we invoke the plugin event handler and
everything looks good to that point, so I suspect a bug in Flash
on Windows (it works on Linux and OSX).

Comment 18

[Josh Aas]

Comment on attachment 431011 [details] [diff] [review]
OSX fix for trunk

I'll review the 1.9.2 patch after this has been on trunk for a few days.

Comment 19

[Josh Aas]

Comment on attachment 431009 [details] [diff] [review]
Additional fix for testplugin on OSX [for 1.9.2]

+ifndef __LP64__

There is no 64-bit support on 1.9.2 so you don't need this.

Comment 20

[Mats Palmgren (inactive)]

http://hg.mozilla.org/mozilla-central/rev/c14e08cf3e61

Filed bug 562068 on the Flash plugin for ignoring some mouse clicks on Windows.