[webkit-reviews] review denied: [Bug 185327] [Win] Implement WebPage::handleEditingKeyboardEvent : [Attachment 339603] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 9 19:58:35 PDT 2018


Fujii Hironori <Hironori.Fujii at sony.com> has denied Don Olmstead
<don.olmstead at sony.com>'s request for review:
Bug 185327: [Win] Implement WebPage::handleEditingKeyboardEvent
https://bugs.webkit.org/show_bug.cgi?id=185327

Attachment 339603: Patch

https://bugs.webkit.org/attachment.cgi?id=339603&action=review




--- Comment #5 from Fujii Hironori <Hironori.Fujii at sony.com> ---
Comment on attachment 339603
  --> https://bugs.webkit.org/attachment.cgi?id=339603
Patch

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

Thank you for the review, Alexey.

>> Source/WebKit/ChangeLog:3
>> +	    [Win] Implement WebPage::handleEditingKeyboardEvent
> 
> Which tests start to pass with this change? I assume this only attempts to
make the very basics work, no Asian text input.

This change just fix the linkage error of WinCairo WebKit2.
WinCairo WebKit2 doesn't have WebKitTestRunner and WebKitAPITests running yet.

>> Source/WebKit/WebProcess/WebPage/win/WebPageWin.cpp:271
>> +	if (!keyEvent || keyEvent->isSystemKey()) // do not treat this as text
input if it's a system key event
> 
> WebKit coding style calls for full sentences, with capitalization and period.

Will do.

>> Source/WebKit/WebProcess/WebPage/win/WebPageWin.cpp:287
>> +	if (event->charCode() < ' ')
> 
> Not even tabs?

This code is copied from Win port WK1.
https://trac.webkit.org/browser/webkit/trunk/Source/WebKitLegacy/win/WebView.cp
p?rev=231622#L2347

And, GTK port WebKit2 has the same code.
https://trac.webkit.org/browser/webkit/trunk/Source/WebKit/WebProcess/WebCoreSu
pport/gtk/WebEditorClientGtk.cpp#L99

It's a complicated issue how to tab in browsers.
https://superuser.com/questions/67934/typing-the-tab-character-in-browser-text-
boxes


More information about the webkit-reviews mailing list