[Webkit-unassigned] [Bug 195225] Check whether to launch a default action instead of action sheet

[bugzilla-daemon at webkit.org]

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

Daniel Bates <dbates at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dbates at webkit.org

--- Comment #3 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 363375
  --> https://bugs.webkit.org/attachment.cgi?id=363375
Patch

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

This patch doesn’t look correct. It contains computations that are not used and that is just weird.

> Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm:285
> +    NSURL *targetURL = _positionInformation->url;

auto?

> Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm:292
> +    RetainPtr<NSMutableDictionary> extendedContext;

Too far from its use. Please move.

> Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm:297
> +        textAtSelection = [delegate selectedTextForActionSheetAssistant:self];

We don’t even use textAtSelection. Just assign to it. Why have it then?

> Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm:299
> +        extendedContext = adoptNS([@{ getkDataDetectorsLeadingText() : _positionInformation->textBefore, getkDataDetectorsTrailingText() : _positionInformation->textAfter } mutableCopy]);

auto extendedContent = ...
(If you take my suggestion and remove line 292)

> Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm:302
> +        context = extendedContext.get();

What is going on here? |context| is not used after this point. So, why are we doing all this work to compute it?

-- 
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/20190302/9f71df80/attachment.html>