[webkit-reviews] review denied: [Bug 15057] EditorClientGtk is missing some important keypress handling, patch attached : [Attachment 16093] fixed changelog and copyright info

[bugzilla-daemon at webkit.org]

Adam Roben <aroben at apple.com> has denied 's request for review:
Bug 15057: EditorClientGtk is missing some important keypress handling, patch
attached
http://bugs.webkit.org/show_bug.cgi?id=15057

Attachment 16093: fixed changelog and copyright info
http://bugs.webkit.org/attachment.cgi?id=16093&action=edit

------- Additional Comments from Adam Roben <aroben at apple.com>
+static const KeyEntry keyEntries[] = {

I think we might as well move this whole array inside interpretKeyEvent, since
it's only used in there.

+const char* Editor::interpretKeyEvent(const KeyboardEvent* evt)

Since we're touching this code anyway, I think it would be good to make this a
static helper function instead of a method on Editor, and perhaps give it a
better name like "commandForEvent".

+#if !PLATFORM(MAC)
+    const char* interpretKeyEvent(const KeyboardEvent* evt);
     void handleKeypress(KeyboardEvent*);
+#endif

handleKeypress should not be within the #if !PLATFORM(MAC) block.
handleKeypress exists on all platforms, but it has two implementations, one in
Editor.cpp, and one in EditorMac.mm.

You should remove the declarations of WebView::interpretKeyEvent and
WebView::handleEditingKeyboardEvent from WebView.h.

It would also be good to be slightly more explicit in your ChangeLogs by saying
that the code was moved from WebView to Editor.