Bug 214424 – iPad cursor is sometimes slow to change to I-beam (e.g. on reddit.com)

• Component: New Bugs
• Status: RESOLVED FIXED

[Tim Horton]

iPad cursor is sometimes slow to change to I-beam (e.g. on reddit.com)

[Tim Horton]

Created attachment 404482 [details]
Patch

[Tim Horton]

<rdar://problem/59503572>

[Tim Horton]

One thing to note that I forgot to include in the changelog: it's OK that we don't request nodeAtPositionHasDoubleClickHandler be filled in for cursor updates, because trackpad touches will not be allowed to trigger the only gesture recognizer that reads from this bit, AND, the code that does read from it uses a isValid check, which will correctly return NO if the position information was requested without that data.

[Wenson Hsieh]

Comment on attachment 404482 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=404482&action=review

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8675
> +            auto deferredRequest = WTFMove(_deferredCursorInteractionRequest);

Nit - I _think_ we prefer std::exchange for cases like this, where the intention is to reset the thing being moved.

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2950
> +        info.nodeAtPositionHasDoubleClickHandler = m_page->mainFrame().nodeRespondingToDoubleClickEvent(request.point, adjustedPoint);

I wonder if `nodeAtPositionHasDoubleClickHandler` should be an Optional<bool> (or maybe a Yes/No/🤷🏻‍♂️ enum class).

[Tim Horton]

(In reply to Wenson Hsieh from comment #4)
> Comment on attachment 404482 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=404482&action=review
> 
> > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8675
> > +            auto deferredRequest = WTFMove(_deferredCursorInteractionRequest);
> 
> Nit - I _think_ we prefer std::exchange for cases like this, where the
> intention is to reset the thing being moved.

Right! I never folded that adjustment into my brain.

> > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2950
> > +        info.nodeAtPositionHasDoubleClickHandler = m_page->mainFrame().nodeRespondingToDoubleClickEvent(request.point, adjustedPoint);
> 
> I wonder if `nodeAtPositionHasDoubleClickHandler` should be an
> Optional<bool> (or maybe a Yes/No/🤷🏻‍♂️ enum class).

I *almost* Optional<>'d it, but oddly that is not how the other optional things work (instead, they depend on people doing isValid checks). But maybe it's best to just do it, to avoid confusion later.

[Tim Horton]

Created attachment 404489 [details]
Patch

[EWS]

Committed r264484: <https://trac.webkit.org/changeset/264484>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404489 [details].