Bug 167565

Summary: [iOS] Expose WebCore::DataDetection::detectContentInRange WKWebProcessPlugInRangeHandle
Product: WebKit Reporter: mitz
Component: WebKit APIAssignee: mitz
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Add -[WKWebProcessPlugInRangeHandle detectDataWithTypes:context:] sam: review+

Description mitz 2017-01-29 14:22:17 PST
A bundle API client requires the ability to “linkify” a range. Patch forthcoming.
Comment 1 mitz 2017-01-29 15:40:18 PST
Created attachment 300075 [details]
Add -[WKWebProcessPlugInRangeHandle detectDataWithTypes:context:]
Comment 2 WebKit Commit Bot 2017-01-29 15:43:22 PST
Attachment 300075 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/API/Cocoa/WKDataDetectorTypes.h:46:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Sam Weinig 2017-01-29 16:01:15 PST
Comment on attachment 300075 [details]
Add -[WKWebProcessPlugInRangeHandle detectDataWithTypes:context:]

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

> Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInRangeHandle.h:46
> +#if TARGET_OS_IPHONE
> +- (NSArray *)detectDataWithTypes:(WKDataDetectorTypes)types context:(NSDictionary *)context WK_API_AVAILABLE(ios(WK_IOS_TBA));
> +#endif

Given that the underlying DataDetection::detectContentInRange() is cross platform, can we make this cross-platform as well?
Comment 4 mitz 2017-01-29 16:09:08 PST
(In reply to comment #3)
> Comment on attachment 300075 [details]
> Add -[WKWebProcessPlugInRangeHandle detectDataWithTypes:context:]
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=300075&action=review
> 
> > Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInRangeHandle.h:46
> > +#if TARGET_OS_IPHONE
> > +- (NSArray *)detectDataWithTypes:(WKDataDetectorTypes)types context:(NSDictionary *)context WK_API_AVAILABLE(ios(WK_IOS_TBA));
> > +#endif
> 
> Given that the underlying DataDetection::detectContentInRange() is cross
> platform, can we make this cross-platform as well?

Thanks for the review, Sam! On macOS, DataDetection::detectContentInRange() has a degenerate implementation that returns nil. I am afraid that it would just be misleading to expose it via the WebKit API.
Comment 5 mitz 2017-01-29 16:28:10 PST
Committed <https://trac.webkit.org/r211354>.
Comment 6 Sam Weinig 2017-01-29 16:29:14 PST
(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 300075 [details]
> > Add -[WKWebProcessPlugInRangeHandle detectDataWithTypes:context:]
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=300075&action=review
> > 
> > > Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInRangeHandle.h:46
> > > +#if TARGET_OS_IPHONE
> > > +- (NSArray *)detectDataWithTypes:(WKDataDetectorTypes)types context:(NSDictionary *)context WK_API_AVAILABLE(ios(WK_IOS_TBA));
> > > +#endif
> > 
> > Given that the underlying DataDetection::detectContentInRange() is cross
> > platform, can we make this cross-platform as well?
> 
> Thanks for the review, Sam! On macOS, DataDetection::detectContentInRange()
> has a degenerate implementation that returns nil. I am afraid that it would
> just be misleading to expose it via the WebKit API.

Indeed!