Bug 72358 - [Qt] Mouse move event not working with Flash 11 on Mac
Summary: [Qt] Mouse move event not working with Flash 11 on Mac
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.6
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 72014 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-11-14 23:27 PST by Titta Heikkala
Modified: 2013-04-18 21:44 PDT (History)
7 users (show)

See Also:


Attachments
html file for testing (122 bytes, text/html)
2011-11-14 23:27 PST, Titta Heikkala
no flags Details
Preliminary fix for the problem based on Qt 4.8 (37.56 KB, patch)
2012-09-17 02:48 PDT, andy.shaw
no flags Details | Formatted Diff | Diff
Patch against trunk (38.04 KB, patch)
2012-09-19 04:46 PDT, andy.shaw
no flags Details | Formatted Diff | Diff
Fixed patch with code style changes against trunk (38.06 KB, patch)
2012-09-19 05:15 PDT, andy.shaw
no flags Details | Formatted Diff | Diff
Updated patch with the typos in the change log fixed (38.06 KB, patch)
2012-09-24 04:02 PDT, andy.shaw
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Titta Heikkala 2011-11-14 23:27:09 PST
Overview:
Mouse move events aren't forwarded to the Flash Player 11 from Webkit on Mac OS. 

Steps to reproduce:
For example, this page: http://www.kirupa.com/developer/mx/animation/slider_tutorial_part2.swf works fine with Flash Player 10.3.183.11, but not with the latest Flash Player 11 (11.1.102.55) when viewed with fancybrowser. Attached also a file that has the page as src to view it with fancybrowser.

Actual results:
With the Flash Player 11 the slider won't work.

Expected results:
The slider can be moved.

Environment:
Mac OS X, Qt 4.7.4 (configured with -universal)
Comment 1 Titta Heikkala 2011-11-14 23:27:51 PST
Created attachment 115106 [details]
html file for testing
Comment 2 Alessandro La Piana 2011-11-16 06:26:11 PST
I was able to reproduce it on Mac OS X 10.7.2
Comment 3 Anton Patiev 2011-11-23 09:55:11 PST
Reproduced on the following environment:
* Mac OS X 10.6
* Qt 4.7.4
* Flash Player 11.0.1.152
Comment 4 Anton Patiev 2011-11-24 09:54:56 PST
Also reproduced on QtWebKit-2.2.0 using QtTestBrowser (/Tools/Scripts/run-launcher --qt).
Comment 5 Alexis Menard (darktears) 2011-11-29 18:27:56 PST
*** Bug 72014 has been marked as a duplicate of this bug. ***
Comment 6 Alessandro La Piana 2011-11-30 05:53:46 PST
After a more detailed analysis it seems the problem is a regression in Flash Player 11 that does not handle the the null event in the Carbon even model that, apparently, is the only way used to track the mouse movement. Since the Carbon event model is deprecated it's unlikely Adobe will fix this. 
It will probably be necessary to rewrite PluginViewMac.mm switching to Cocoa event model.
Comment 7 andy.shaw 2012-09-17 02:48:42 PDT
Created attachment 164361 [details]
Preliminary fix for the problem based on Qt 4.8

Additionally this patch solves a number of issues regarding indirect rendering, signal emission to indicate the frame needs repainting and 64bit support as well as Cocoa support on Mac.
Comment 8 Simon Hausmann 2012-09-18 05:02:51 PDT
Comment on attachment 164361 [details]
Preliminary fix for the problem based on Qt 4.8

View in context: https://bugs.webkit.org/attachment.cgi?id=164361&action=review

Nice. Here is some initial feedback :). I suggest to rebase this against webkit trunk for official submission with ChangeLog and corrected paths (no src/3rdparty/webkit). You can also use Tools/Scripts/check-webkit-style to do some rudimentary style checking.

> src/3rdparty/webkit/Source/WebCore/plugins/PluginView.cpp:1282
> +static const char* ChromeUserAgent = "Mozilla/5.0 ("

This should be static const char* const ChromeUserAgent or static const char ChromeUserAgent[] - either way the variable should be const, too, not only the characters it points to.

> src/3rdparty/webkit/Source/WebCore/plugins/PluginView.h:233
> +        bool popUpContextMenu(NPMenu *menu);

Coding style nitpick: The * should be placed near the type instead of the name.

> src/3rdparty/webkit/Source/WebCore/plugins/mac/PluginViewMac.mm:154
> +static void InitializeNPCocoaEvent(NPCocoaEvent* event)

Initialize* -> initialize*

> src/3rdparty/webkit/Source/WebCore/plugins/mac/PluginViewMac.mm:156
> +  memset(event, 0, sizeof(NPCocoaEvent));

Incorrect level of indentation.

> src/3rdparty/webkit/Source/WebCore/plugins/mac/PluginViewMac.mm:402
> +        if (NPEventModelCocoa != m_eventModel)

I think m_eventModel != NPEventModelCocoa would be more consistent in style than NPEventModelCocoa != m_eventModel :)

> src/3rdparty/webkit/Source/WebCore/plugins/mac/PluginViewMac.mm:482
> +        NP_CGContext *miniContext = new NP_CGContext;
> +        miniContext->context = this->m_contextRef;
> +        m_npWindow.window = (void*)miniContext;

Why can't you use m_npCgContext here?

> src/3rdparty/webkit/Source/WebCore/plugins/mac/PluginViewMac.mm:558
> +    QPainter *p = context->platformContext();

Coding style nitpick: * placement

> src/3rdparty/webkit/Source/WebCore/plugins/mac/PluginViewMac.mm:568
> +        if (!cgContext) {
> +            return;
> +        } else {

Single line bodies don't use braces.

> src/3rdparty/webkit/Source/WebCore/plugins/mac/PluginViewMac.mm:574
> +    } else {
> +        m_usePixmap = false;
> +    }

Ditto.

> src/3rdparty/webkit/Source/WebCore/plugins/mac/PluginViewMac.mm:773
> +        NSEvent *currentEvent = [NSApp currentEvent];

Coding style nitpick: * placement
Comment 9 andy.shaw 2012-09-19 04:46:05 PDT
Created attachment 164714 [details]
Patch against trunk

Followed the steps to build and test this against trunk, I think that I covered everything.
Comment 10 WebKit Review Bot 2012-09-19 05:07:01 PDT
Attachment 164714 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/plugins/npapi.h:418:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/plugins/npapi.h:420:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/ChangeLog:4:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 5 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 andy.shaw 2012-09-19 05:15:12 PDT
Created attachment 164720 [details]
Fixed patch with code style changes against trunk

The npapi.h file has a couple of code style oddities because it is done to match the rest of the file.  If this should change then I can do that.
Comment 12 WebKit Review Bot 2012-09-19 05:24:06 PDT
Attachment 164720 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/plugins/npapi.h:418:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/plugins/npapi.h:420:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Jiang Jiang 2012-09-22 13:48:22 PDT
Regarding the title "[Qt] Implement support for Cooca based NAPAI plugins on Mac", should it be NPAPI?
Comment 14 andy.shaw 2012-09-23 12:22:41 PDT
Good point, that is a typo on my side, I can rectify that quickly enough.
Comment 15 Jiang Jiang 2012-09-23 12:26:35 PDT
(In reply to comment #14)
> Good point, that is a typo on my side, I can rectify that quickly enough.

And Cooca -> Cocoa as well :)
Comment 16 andy.shaw 2012-09-24 04:02:17 PDT
Created attachment 165348 [details]
Updated patch with the typos in the change log fixed
Comment 17 WebKit Review Bot 2012-09-24 04:04:35 PDT
Attachment 165348 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/plugins/npapi.h:418:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/plugins/npapi.h:420:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 Bobber 2012-11-13 01:55:46 PST
Hi Andy,

I applyied your patch qtwebkit_23 and changed code
'JSC::JSLock::DropAllLocks dropAllLocks(JSC::SilenceAssertionsOnly); ' in Source/WebCore/plugins/mac/PluginViewMac.mm to 'JSC::JSLock::DropAllLocks dropAllLocks(JSDOMWindowBase::commonJSGlobalData());' because I got error that JSC::SilenceAssertionsOnly is not found on Mac. page: http://www.kirupa.com/developer/mx/animation/slider_tutorial_part2.swf works fine now. Could you please confirmed the change is ok?

Thanks,
Bobber
Comment 19 andy.shaw 2012-11-13 23:52:38 PST
Bobber: I am afraid I don't know as I don't know that part of webkit well enough, maybe someone else can comment on that.
Comment 20 Simon Hausmann 2012-11-29 05:25:24 PST
Comment on attachment 165348 [details]
Updated patch with the typos in the change log fixed

View in context: https://bugs.webkit.org/attachment.cgi?id=165348&action=review

> Source/WebCore/plugins/mac/PluginViewMac.mm:1078
> +    JSC::JSLock::DropAllLocks dropAllLocks(JSC::SilenceAssertionsOnly);

Will fix this when landing, the API was changed just recently.
Comment 21 Simon Hausmann 2012-11-29 05:42:16 PST
Committed r136124: <http://trac.webkit.org/changeset/136124>
Comment 22 Bobber 2013-04-18 21:44:56 PDT
Hi guys, could you please check bug 113553?