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
Don Olmstead
2018-05-04 14:58:49 PDT
Created attachment 339599 [details]
Patch
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.
Created attachment 339603 [details]
Patch
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? 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 Created attachment 340065 [details]
Patch
@ap, could you review again? Could anyone review this? Created attachment 340481 [details]
Patch
Comment on attachment 340481 [details] Patch Clearing flags on attachment: 340481 Committed r231853: <https://trac.webkit.org/changeset/231853> All reviewed patches have been landed. Closing bug. |