[Webkit-unassigned] [Bug 138720] Refactor iOS selection gestures

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=138720

--- Comment #3 from Benjamin Poulain <benjamin at webkit.org> ---
Comment on attachment 241522
  --> https://bugs.webkit.org/attachment.cgi?id=241522
Patch

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

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1786
> +    _page->selectPositionAtPoint(WebCore::IntPoint(point), [selectionHandler](CallbackBase::Error error) {
> +        selectionHandler();

No need to account for errors? (same for the ones below).

For example, if the WebProcess dies during a gesture, you may want to fallback safely?

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:2658
> +    [self reloadInputViews];
> +    

This change is odd. I would expect [reloadInputViews] to be the last thing done after updating the internal state.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:303
> +    , m_selectionBaseIsStart(false)

This should be along the other state booleans of WebPage.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:1617
> +    m_selectionBaseIsStart = baseIsStart; // FIXME: do we need to flip this for RTL?
> +    send(Messages::WebPageProxy::UnsignedCallback(baseIsStart, callbackID));

This does not look right.

There is one message just to set a boolean state on WebPage, but that boolean is only used when interpreting other messages.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:1641
> +            m_selectionBaseIsStart = !m_selectionBaseIsStart;

m_selectionBaseIsStart = false;

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:1650
> +            m_selectionBaseIsStart = !m_selectionBaseIsStart;

m_selectionBaseIsStart = true;

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141114/5b6ad6bf/attachment-0002.html>