Correctly set up context for Data Detectors

[Megan Gardner]

Correctly set up context for Data Detectors

[Megan Gardner]

Created attachment 393922 [details]
Patch

[Tim Horton]

Comment on attachment 393922 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm:744
> +    [ddContextMenuActionClass updateContext:context withSourceRect:sourceRect];

You need to use the return value of this method, it doesn't/can't mutate the one passed in

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6754
> +    [ddContextMenuActionClass updateContext:context.get() withSourceRect:frameInContainerViewCoordinates];

You need to use the return value of this method, it doesn't/can't mutate the one passed in

[Megan Gardner]

Created attachment 393926 [details]
Patch

[Tim Horton]

Comment on attachment 393926 [details]
Patch

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

> Source/WebKit/ChangeLog:5
> +

where's the rdar

> Source/WebKit/ChangeLog:8
> +        Not passing in a nil context, and running the formatter as requested by Data Detectors team.

This is an odd sentence fragment.

I would write something more like:

"Stop passing in a nil context. This is OK now, but won't be in the near future.
Also, call -updateContext:withSourceRect: so that DataDetectors can populate the context appropriately."

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6752
>      CGRect frameInContainerViewCoordinates = [self convertRect:sourceRect toView:self.containerForContextMenuHintPreviews];

Here, we're converting into the coordinate space of containerForContextMenuHintPreviews. But I think that sometimes this method is used to provide context for different views? Maybe we need to pass in the view whose coordinate space we should use as an argument to this method?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6753
> +    auto ddContextMenuActionClass = getDDContextMenuActionClass();

No need for this local, in this case.

[Megan Gardner]

Created attachment 393932 [details]
Patch

[EWS]

Committed r258684: <https://trac.webkit.org/changeset/258684>

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

[Radar WebKit Bug Importer]

<rdar://problem/60623947>

[Megan Gardner]

<rdar://problem/60612327>