[Webkit-unassigned] [Bug 81950] Add Android keycodes

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Bolin Hsu <bhsu at google.com>  2012-03-26 09:44:22 PST ---
(In reply to comment #4)
> (From update of attachment 133396 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133396&action=review
> 
> Thanks! I've got some questions/comments on the patch (following up on the patch failing to apply in the e-mail thread).
> 
> > WebKit/Source/WebCore/ChangeLog:3
> > +        Add Android keycodes
> 
> nit: Since this is Chromium-specific code, we usually prepend the title with [Chromium].
I will change the title, if this is possible. 

> 
> > WebKit/Source/WebCore/platform/chromium/KeyCodeConversionAndroid.cpp:36
> > +namespace {
> 
> The usage of anonymous namespaces within WebKit is not really preferred (though not completely disallowed either). What is the benefit of putting these values in one? It's going to break during a future NDK update either way.
The intention of the anonymous namespace is to limit the visibility of the enum to this file. I will remove it.

> 
> > WebKit/Source/WebCore/platform/chromium/KeyCodeConversionAndroid.cpp:238
> > +#if defined(GTV)
> 
> What is the reason that we want to keep this specific to GTV?
The guarded part uses GTV specific VKEY_OEM_103 and VKEY_OEM_104 defined elsewhere. I will take this chance to add the definition of these two codes, then this is not GTV specific anymore.

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