[Webkit-unassigned] [Bug 48510] [GTK] Implement WebEventFactory, WebErrors classes for WebKit2

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #83283|review?                     |review+
               Flag|                            |




--- Comment #32 from Martin Robinson <mrobinson at webkit.org>  2011-02-22 08:00:49 PST ---
(From update of attachment 83283)
View in context: https://bugs.webkit.org/attachment.cgi?id=83283&action=review

Great! I have a few consistency nits below.

> Source/WebCore/platform/PlatformKeyboardEvent.h:182
> +        static String keyIdentifierForGdkKeyCode(guint);
> +        static int windowsKeyCodeForGdkKeyCode(unsigned int);
> +        static String singleCharacterString(guint);

Please use either allk "guint" here or all "unsigned int" or all "unsigned." Change singleCharacterString to singleCharacterStringForGdkKeyCode for consistency.

> Source/WebKit2/Shared/gtk/WebEventFactory.cpp:42
> +static inline bool isKeyCodeFromKeyPad(unsigned keyval)

Just for consistency, I think this should be isGdkKeyCodeFromKeyPad(<guint or unsigned int or unsigned> keyCode)

> Source/WebKit2/Shared/gtk/WebEventFactory.cpp:122
> +    return WebMouseEvent(type, buttonForEvent(event), IntPoint(x, y), IntPoint(xRoot, yRoot), 0 /* deltaX */, 0 /* delta Y*/,
> +                         0 /*deltaZ */, currentClickCount, static_cast<WebEvent::Modifiers>(0), gdk_event_get_time(event));

Pretty nitful here, but I think it makes sense to break this up to one or two per line for readability. :)

> Source/WebKit2/Shared/gtk/WebEventFactory.cpp:169
> +    return WebKeyboardEvent((event->type == GDK_KEY_RELEASE) ? WebEvent::KeyUp : WebEvent::KeyDown,
> +                            PlatformKeyboardEvent::singleCharacterString(event->keyval),
> +                            PlatformKeyboardEvent::singleCharacterString(event->keyval),
> +                            PlatformKeyboardEvent::keyIdentifierForGdkKeyCode(event->keyval),
> +                            PlatformKeyboardEvent::windowsKeyCodeForGdkKeyCode(event->keyval),
> +                            static_cast<int>(event->keyval), 0 /* macCharCode */, false /* isAutoRepeat */,
> +                            isKeyCodeFromKeyPad(event->keyval), false /* isSystemKey */,
> +                            modifiersForEvent(reinterpret_cast<GdkEvent*>(event)),
> +                            gdk_event_get_time(reinterpret_cast<GdkEvent*>(event)));

Ditto.

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