[Webkit-unassigned] [Bug 72358] [Qt] Mouse move event not working with Flash 11 on Mac

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=72358





--- Comment #8 from Simon Hausmann <hausmann at webkit.org>  2012-09-18 05:02:51 PST ---
(From update of attachment 164361)
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

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.