Bug 139617

Summary: Implement Data Detectors immediate actions for WebKit2
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bdakin, commit-queue, conrad_shultz, eric.carlson, glenn, jer.noble, philipj, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch bdakin: review+

Description Tim Horton 2014-12-13 00:16:00 PST
Implement Data Detectors immediate actions for WebKit2
Comment 1 Tim Horton 2014-12-13 00:16:56 PST
Created attachment 243251 [details]
Patch
Comment 2 Tim Horton 2014-12-13 00:18:07 PST
<rdar://problem/19198539>
Comment 3 WebKit Commit Bot 2014-12-13 00:19:13 PST
Attachment 243251 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/mac/WKImmediateActionController.mm:464:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/mac/WKImmediateActionController.mm:466:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/mac/WKImmediateActionController.mm:470:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 3 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Beth Dakin 2014-12-13 02:01:58 PST
Comment on attachment 243251 [details]
Patch

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

> Source/WebKit2/UIProcess/API/mac/WKView.mm:3615
> +    // FIXME: Temporarily disable action menu installation.

This is pretty shameful.
Comment 5 Eric Carlson 2014-12-13 07:52:16 PST
Comment on attachment 243251 [details]
Patch

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

> Source/WebKit2/UIProcess/mac/WKImmediateActionController.mm:93
> +    _immediateActionRecognizer.enabled = NO;
> +    _immediateActionRecognizer.enabled = YES;

See my comment in the WK1 version of this patch.

> Source/WebKit2/UIProcess/mac/WKImmediateActionController.mm:462
> +    // Ref our WebPageProxy for use in the blocks below.
> +    RefPtr<WebPageProxy> page = _page;

Nit: this seems fairly self evident, is the comment necessary?
Comment 7 Tim Horton 2014-12-15 12:27:37 PST
http://trac.webkit.org/changeset/177303