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

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Peter Beverloo <peter at chromium.org>  2012-03-26 08:50:22 PST ---
(From update of attachment 133396)
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].

> 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.

> WebKit/Source/WebCore/platform/chromium/KeyCodeConversionAndroid.cpp:238
> +#if defined(GTV)

What is the reason that we want to keep this specific to GTV?

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