[Webkit-unassigned] [Bug 62451] [EFL] Move keyIdentifierForEvasKeyName() and windowsKeyCodeForEvasKeyName() to the EflKeyboardUtilities.cpp to use in the WebKit2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 17 18:58:40 PST 2011


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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #115725|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #9 from Martin Robinson <mrobinson at webkit.org>  2011-11-17 18:58:39 PST ---
(From update of attachment 115725)
View in context: https://bugs.webkit.org/attachment.cgi?id=115725&action=review

Seems fine overall, but I have a few style nits.

> Source/WebCore/platform/efl/EflKeyboardUtilities.cpp:33
> +#include "WindowsKeyboardCodes.h"
> +
> +#include <wtf/HashMap.h>

No need for a newline here.

> Source/WebCore/platform/efl/EflKeyboardUtilities.cpp:44
> +static KeyMap gKeyMap;
> +static WindowsKeyMap gWindowsKeyMap;
> +

Static initializers inrease startup time and can lead to tricky errors. It's probably better to remove them. For an example see here: http://trac.webkit.org/changeset/100070

> Source/WebCore/platform/efl/EflKeyboardUtilities.cpp:80
> +    gWindowsKeyMap.set("Return",     VK_RETURN);
> +    gWindowsKeyMap.set("KP_Return",  VK_RETURN);
> +    gWindowsKeyMap.set("Alt_L",      VK_MENU);
> +    gWindowsKeyMap.set("ISO_Level3_Shift", VK_MENU);
> +    gWindowsKeyMap.set("Menu",       VK_MENU);
> +    gWindowsKeyMap.set("Shift_L",    VK_SHIFT);

This kind of alignment is a violation of WebKit style rules.

> Source/WebCore/platform/efl/EflKeyboardUtilities.cpp:125
> +    // Alphabet

Comments should be full sentences with a period at the end.

> Source/WebCore/platform/efl/EflKeyboardUtilities.cpp:132
> +    // Digits

Ditto.

> Source/WebCore/platform/efl/EflKeyboardUtilities.cpp:138
> +    // Shifted digits

Ditto.

> Source/WebCore/platform/efl/EflKeyboardUtilities.cpp:154
> +    // F_XX

Ditto.

> Source/WebCore/platform/efl/PlatformKeyboardEventEfl.cpp:34
>  #include "PlatformKeyboardEvent.h"
>  

No newlines necessary here.

> Source/WebCore/platform/efl/PlatformKeyboardEventEfl.cpp:39
>  #include "TextEncoding.h"
> -#include "WindowsKeyboardCodes.h"
>  
>  #include <Evas.h>

Ditto.

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



More information about the webkit-unassigned mailing list