[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 bugzilla-daemon at webkit.org
Thu Aug 23 09:27:39 PDT 2007

Adam Roben <aroben at apple.com> has denied 's request for review:
Bug 15057: EditorClientGtk is missing some important keypress handling, patch

Attachment 16093: fixed changelog and copyright info

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

+    const char* interpretKeyEvent(const KeyboardEvent* evt);
     void handleKeypress(KeyboardEvent*);

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.

More information about the webkit-reviews mailing list