Bug 200579 - Tapping buttons in Data Detectors lookup previews doesn't work
Summary: Tapping buttons in Data Detectors lookup previews doesn't work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-09 10:17 PDT by Tim Horton
Modified: 2019-08-12 11:42 PDT (History)
8 users (show)

See Also:


Attachments
Patch (7.06 KB, patch)
2019-08-09 10:21 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (7.12 KB, patch)
2019-08-09 10:25 PDT, Tim Horton
jbedard: commit-queue+
Details | Formatted Diff | Diff
Patch (1.12 KB, patch)
2019-08-12 09:58 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2019-08-09 10:17:19 PDT
Tapping buttons in Data Detectors lookup previews doesn't work
Comment 1 Tim Horton 2019-08-09 10:21:39 PDT
Created attachment 375932 [details]
Patch
Comment 2 Tim Horton 2019-08-09 10:21:41 PDT
<rdar://problem/54056519>
Comment 3 Tim Horton 2019-08-09 10:25:44 PDT
Created attachment 375933 [details]
Patch
Comment 4 Megan Gardner 2019-08-09 11:23:33 PDT
Comment on attachment 375933 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8298
> +    if ([configuration isKindOfClass:getDDContextMenuConfigurationClass()]) {

File a bug to clean this all up once the SPI has settled?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8305
> +        shouldExpandPreview = !!ddConfiguration.interactionViewControllerProvider;

I find it odd that the SPI had a var for this, but now does not? But I'm assuming you know what's going on here..
Comment 5 Tim Horton 2019-08-09 11:24:41 PDT
Comment on attachment 375933 [details]
Patch

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

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8298
>> +    if ([configuration isKindOfClass:getDDContextMenuConfigurationClass()]) {
> 
> File a bug to clean this all up once the SPI has settled?

Sure

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8305
>> +        shouldExpandPreview = !!ddConfiguration.interactionViewControllerProvider;
> 
> I find it odd that the SPI had a var for this, but now does not? But I'm assuming you know what's going on here..

They upgraded from a bool to a nullable block that returns a VC 🤷‍♂️
Comment 6 Tim Horton 2019-08-09 12:50:46 PDT
Committed r248469: <https://trac.webkit.org/changeset/248469>
Comment 7 Jonathan Bedard 2019-08-12 09:56:11 PDT
This broke the iOS 13 seed build...we don't have bots for it yet, but I've been building it pretty regularly locally looking for this sort of thing.
Comment 8 Jonathan Bedard 2019-08-12 09:58:00 PDT
Reopening to attach new patch.
Comment 9 Jonathan Bedard 2019-08-12 09:58:01 PDT
Created attachment 376073 [details]
Patch
Comment 10 WebKit Commit Bot 2019-08-12 11:42:26 PDT
Comment on attachment 376073 [details]
Patch

Clearing flags on attachment: 376073

Committed r248535: <https://trac.webkit.org/changeset/248535>
Comment 11 WebKit Commit Bot 2019-08-12 11:42:27 PDT
All reviewed patches have been landed.  Closing bug.