Summary: | Implement support for cursor interactions on iPad | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||||
Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bdakin, benjamin, cdumez, cmarcelo, darin, ews-watchlist, megan_gardner, simon.fraser, webkit-bug-importer, wenson_hsieh | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Tim Horton
2020-03-18 19:46:22 PDT
Created attachment 393938 [details]
Patch
(AFAIK the style bot complaint is only true for macOS, not iOS, so I'm ignoring it) Comment on attachment 393938 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393938&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8122 > + static dispatch_once_t onceToken; > + dispatch_once(&onceToken, ^{ > + // System apps will always be linked on the current OS, so > + // check them before the linked-on-or-after. > + > + // <rdar://problem/59521967> iAd Video does not respond to mouse events, only touch events > + if (WebCore::IOSApplication::isNews() || WebCore::IOSApplication::isStocks()) > + shouldUseMouseGestureRecognizer = NO; > + > + if (WebKit::linkedOnOrAfter(WebKit::SDKVersion::FirstThatSendsNativeMouseEvents)) > + return; > + > + // <rdar://problem/59155220> Some Feedly UI does not respond to mouse events, only touch events > + if (WebCore::IOSApplication::isFeedly()) > + shouldUseMouseGestureRecognizer = NO; > + }); Is the thread safety really needed here? Can it be called from more than one thread concurrently? - If so, is everything the dispatch_once block does safe to be called from any thread? - If not, I suggest instead writing this: static const BOOL shouldUseMouseGestureRecognizer = []() -> BOOL { ... code here ... }(); > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8127 > +- (void)setupMouseGestureRecognizer The verb "set up" is two words, not one. So I would name this setUpMouseGestureRecognizer. > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8154 > +- (void)setupCursorInteraction And name this setUpCursorInteraction. (In reply to Darin Adler from comment #3) > Comment on attachment 393938 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=393938&action=review > > > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8122 > > + static dispatch_once_t onceToken; > > + dispatch_once(&onceToken, ^{ > > + // System apps will always be linked on the current OS, so > > + // check them before the linked-on-or-after. > > + > > + // <rdar://problem/59521967> iAd Video does not respond to mouse events, only touch events > > + if (WebCore::IOSApplication::isNews() || WebCore::IOSApplication::isStocks()) > > + shouldUseMouseGestureRecognizer = NO; > > + > > + if (WebKit::linkedOnOrAfter(WebKit::SDKVersion::FirstThatSendsNativeMouseEvents)) > > + return; > > + > > + // <rdar://problem/59155220> Some Feedly UI does not respond to mouse events, only touch events > > + if (WebCore::IOSApplication::isFeedly()) > > + shouldUseMouseGestureRecognizer = NO; > > + }); > > Is the thread safety really needed here? Can it be called from more than one > thread concurrently? > > - If so, is everything the dispatch_once block does safe to be called from > any thread? > > - If not, I suggest instead writing this: > > static const BOOL shouldUseMouseGestureRecognizer = []() -> BOOL { > ... code here ... > }(); ... good point. > > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8127 > > +- (void)setupMouseGestureRecognizer > > The verb "set up" is two words, not one. So I would name this > setUpMouseGestureRecognizer. This file is alllll mixed up about the capitalization of setUp, so I'll be bold and fix the whole thing :) Created attachment 394003 [details]
Patch
Created attachment 394004 [details]
Patch
|