Bug 185327

Summary:

[Win] Implement WebPage::handleEditingKeyboardEvent

Product:

WebKit

Reporter:

Don Olmstead <don.olmstead>

Component:

WebKit Misc.

Assignee:

Fujii Hironori <fujii.hironori>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, bfulgham, commit-queue, ews-watchlist, fujii.hironori, pvollan, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

174003

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Don Olmstead

Reported 2018-05-04 14:58:49 PDT

Provide an implementation

Attachments
Patch (2.56 KB, patch) 2018-05-04 15:01 PDT, Don Olmstead	no flags	Details Formatted Diff Diff
Patch (2.56 KB, patch) 2018-05-04 15:37 PDT, Don Olmstead	no flags	Details Formatted Diff Diff
Patch (2.71 KB, patch) 2018-05-09 20:12 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (2.79 KB, patch) 2018-05-16 01:51 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Don Olmstead

Comment 1 2018-05-04 15:01:03 PDT

Created attachment 339599 [details] Patch

EWS Watchlist

Comment 2 2018-05-04 15:03:26 PDT

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

Comment 3 2018-05-04 15:37:20 PDT

Created attachment 339603 [details] Patch

Alexey Proskuryakov

Comment 4 2018-05-05 16:14:23 PDT

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 5 2018-05-09 19:58:35 PDT

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

Comment 6 2018-05-09 20:12:48 PDT

Created attachment 340065 [details] Patch

Fujii Hironori

Comment 7 2018-05-13 18:32:49 PDT

@ap, could you review again?

Fujii Hironori

Comment 8 2018-05-14 18:44:59 PDT

Could anyone review this?

Fujii Hironori

Comment 9 2018-05-16 01:51:50 PDT

Created attachment 340481 [details] Patch