Bug 62802 – [GTK] Fix typos in PageClientImpl::getEditorCommandsForKeyEvent()

• Component: WebKit2
• Status: RESOLVED DUPLICATE

[Carlos Garcia Campos]

There are two typos there, one in the assert command that makes the build fail with --enable-debug and another one in the line to get the the even type.

[Carlos Garcia Campos]

Created attachment 97461 [details]
Patch

It also renames KeyPress as KeyUp since it's confusing to have KeyDown and KeyPress, I would use KeyPress/KeyRelease or KeyDown/KeyUp

[Martin Robinson]

Comment on attachment 97461 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=97461&action=review

> Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp:62
> +    ASSERT(event.type() == WebEvent::KeyDown || event.type() == WebEvent::KeyUp);
> +    KeyBindingTranslator::EventType type = event.type() == WebEvent::KeyDown ?
> +        KeyBindingTranslator::KeyDown : KeyBindingTranslator::KeyUp;
>      m_keyBindingTranslator.getEditorCommandsForKeyEvent(const_cast<GdkEventKey*>(&event.nativeEvent()->key), type, commandList);

The typo fix looks good, but a  WebEvent::KeyDown and a WebEvent.KeyPress event are two very different things.

[Carlos Garcia Campos]

Comment on attachment 97461 [details]
Patch

Clearing flags, martin explained to me that keypress and keyup are actually two different things.

[Carlos Garcia Campos]

Created attachment 98487 [details]
New patch

Looking at http://trac.webkit.org/browser/trunk/Source/WebCore/dom/KeyboardEvent.cpp#L37 it seems KeyPress is WebEvent::Char.

[Martin Robinson]

Comment on attachment 98487 [details]
New patch

View in context: https://bugs.webkit.org/attachment.cgi?id=98487&action=review

> Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp:61
> +    ASSERT(event.type() == WebEvent::KeyDown || event.type() == WebEvent::Char);
> +    KeyBindingTranslator::EventType type = event.type() == WebEvent::KeyDown ?

I'm not sure I understand here when we generate char events on the UIProcess side and why they map to KeyDown DOM events.

[Martin Robinson]

Dupe of https://bugs.webkit.org/show_bug.cgi?id=63081 now?

[Carlos Garcia Campos]

*** This bug has been marked as a duplicate of bug 63081 ***

[Daniel Bates]

Comment on attachment 98487 [details]
New patch

Clearing review flag to get this out of the review queue since this bug was marked as a duplicate of bug #63081. At the time of writing, bug #63081 has a patch up for review.