Bug 185327 – [Win] Implement WebPage::handleEditingKeyboardEvent

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Don Olmstead]

Provide an implementation

[Don Olmstead]

Created attachment 339599 [details]
Patch

[EWS Watchlist]

Attachment 339599 [details] did not pass style-queue:


ERROR: Source/WebKit/WebProcess/WebPage/win/WebPageWin.cpp:271:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Don Olmstead]

Created attachment 339603 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 339603 [details]
Patch

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

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

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

> Source/WebKit/WebProcess/WebPage/win/WebPageWin.cpp:287
> +    // Don't insert null or control characters as they can result in unexpected behaviour
> +    if (event->charCode() < ' ')

Not even tabs?

[Fujii Hironori]

Comment on attachment 339603 [details]
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.cpp?rev=231622#L2347

And, GTK port WebKit2 has the same code.
https://trac.webkit.org/browser/webkit/trunk/Source/WebKit/WebProcess/WebCoreSupport/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

[Fujii Hironori]

Created attachment 340065 [details]
Patch

[Fujii Hironori]

@ap, could you review again?

[Fujii Hironori]

Could anyone review this?

[Fujii Hironori]

Created attachment 340481 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 340481 [details]
Patch

Clearing flags on attachment: 340481

Committed r231853: <https://trac.webkit.org/changeset/231853>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/40299696>