Bug 209268 – Implement support for cursor interactions on iPad

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tim Horton]

Implement support for cursor interactions on iPad

[Tim Horton]

Created attachment 393938 [details]
Patch

[Tim Horton]

(AFAIK the style bot complaint is only true for macOS, not iOS, so I'm ignoring it)

[Darin Adler]

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.

[Tim Horton]

(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 :)

[Tim Horton]

Created attachment 394003 [details]
Patch

[Tim Horton]

Created attachment 394004 [details]
Patch

[Tim Horton]

https://trac.webkit.org/changeset/258721/webkit

[Radar WebKit Bug Importer]

<rdar://problem/60645044>