Bug 185327

Summary: [Win] Implement WebPage::handleEditingKeyboardEvent
Product: WebKit Reporter: Don Olmstead <don.olmstead>
Component: WebKit Misc.Assignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, commit-queue, ews-watchlist, Hironori.Fujii, pvollan, webkit-bug-importer
Priority: P2 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
Patch (2.56 KB, patch)
2018-05-04 15:37 PDT, Don Olmstead
no flags
Patch (2.71 KB, patch)
2018-05-09 20:12 PDT, Fujii Hironori
no flags
Patch (2.79 KB, patch)
2018-05-16 01:51 PDT, Fujii Hironori
no flags
Don Olmstead
Comment 1 2018-05-04 15:01:03 PDT
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
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
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
WebKit Commit Bot
Comment 10 2018-05-16 10:54:56 PDT
Comment on attachment 340481 [details] Patch Clearing flags on attachment: 340481 Committed r231853: <https://trac.webkit.org/changeset/231853>
WebKit Commit Bot
Comment 11 2018-05-16 10:54:58 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 12 2018-05-16 10:55:29 PDT
Note You need to log in before you can comment on or make changes to this bug.