Bug 200579

Summary: Tapping buttons in Data Detectors lookup previews doesn't work
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, dino, jbedard, megan_gardner, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
jbedard: commit-queue+
Patch none

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.