| Summary: | Use synchronous messages for keyEvent on iOS | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Enrica Casucci <enrica> | ||||
| Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | CLOSED WONTFIX | ||||||
| Severity: | Normal | CC: | ap | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | iPhone / iPad | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Enrica Casucci
2014-10-20 17:38:44 PDT
Created attachment 240164 [details]
Patch
Comment on attachment 240164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240164&action=review Oh noes :( > Source/WebKit2/UIProcess/WebPageProxy.cpp:1582 > +void WebPageProxy::handleKeyboardEvent(const NativeWebKeyboardEvent& event, bool useSynchronousMessage) Can this be made into an entirely separate code path? This code is so complicated that mixing sync and async code paths seems very confusing. Or at the very least, the argument should be named, not a cryptic "true/false". Comment on attachment 240164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240164&action=review >> Source/WebKit2/UIProcess/WebPageProxy.cpp:1582 >> +void WebPageProxy::handleKeyboardEvent(const NativeWebKeyboardEvent& event, bool useSynchronousMessage) > > Can this be made into an entirely separate code path? This code is so complicated that mixing sync and async code paths seems very confusing. > > Or at the very least, the argument should be named, not a cryptic "true/false". I don't think the code is that complicated and I did not want any duplication. The function has a default value of false, so all the other call sites don't even need to know about this parameter. The only place where this is used is in WKContentViewInteraction where it is passed [self useSynchronousKeyEvent], which is pretty clear. Comment on attachment 240164 [details]
Patch
Having discussed this in person, we'd like to try a different approach.
We opted for a different solution to the problem. We are not going to pursue this. |