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

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from EunMi Lee <eunmi15.lee at samsung.com>  2011-11-17 21:14:55 PST ---
I'm very grateful for your kind review!

(In reply to comment #9)
> (From update of attachment 115725 [details])
> 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.
I removed newline.
> 
> > 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
I replaced global static variables with local ones as an example that you mentioned. Thanks for your kind guidance.

> 
> > 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.
> 
I replaced all comments with full sentences.

> > 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.
newlines are removed.

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