* SUMMARY Update the default action sheets for links to include an "Open in “App Name"" action in addition to the usual "Open" action for links that have an associated app link.
<rdar://problem/21221902>
Created attachment 256249 [details] [PATCH] Proposed Fix
Comment on attachment 256249 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=256249&action=review > Source/WebKit2/UIProcess/API/Cocoa/_WKElementAction.h:40 > + _WKElementActionTypeOpenInExternalApplication, We could wrap this enum value in a HAVE(APP_LINKS) guard, kind of like the AddToReadingList value below. Let me know what you think.
Comment on attachment 256249 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=256249&action=review The patch looks good to me. >> Source/WebKit2/UIProcess/API/Cocoa/_WKElementAction.h:40 >> + _WKElementActionTypeOpenInExternalApplication, > > We could wrap this enum value in a HAVE(APP_LINKS) guard, kind of like the AddToReadingList value below. Let me know what you think. That sounds right.
(In reply to comment #4) > Comment on attachment 256249 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=256249&action=review > > The patch looks good to me. > > >> Source/WebKit2/UIProcess/API/Cocoa/_WKElementAction.h:40 > >> + _WKElementActionTypeOpenInExternalApplication, > > > > We could wrap this enum value in a HAVE(APP_LINKS) guard, kind of like the AddToReadingList value below. Let me know what you think. > > That sounds right. We can’t use the HAVE macro in a framework header.
Comment on attachment 256249 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=256249&action=review > Source/WebCore/English.lproj/Localizable.strings:413 > +"Open in External Application ActionSheet Link" = "Open in â%@â"; This is not an appropriate key. The key should generally be the same as the value. If there’s ambiguity, the key should be the same as the value plus a disambiguating parenthetical, but in this case there’s no ambiguity.
Comment on attachment 256249 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=256249&action=review >>>> Source/WebKit2/UIProcess/API/Cocoa/_WKElementAction.h:40 >>>> + _WKElementActionTypeOpenInExternalApplication, >>> >>> We could wrap this enum value in a HAVE(APP_LINKS) guard, kind of like the AddToReadingList value below. Let me know what you think. >> >> That sounds right. > > We can’t use the HAVE macro in a framework header. An availability annotation is required here. > Source/WebKit2/UIProcess/ApplicationStateTracker.h:44 > + bool hasEntitlement(NSString *entitlement) const; Why is this a member function?
Comment on attachment 256249 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=256249&action=review > Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:89 > +static _WKElementAction *openInExternalApplicationElementActionForURL(NSURL *url) > +{ > + if (!applicationHasCanGetAppLinkEntitlement()) > + return nil; > + > + __block _WKElementAction *action = nil; > + > + dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); > + [LSAppLink getAppLinkWithURL:url completionHandler:^(LSAppLink *appLink, NSError *error) { > + if (appLink) { > + NSString *localizedAppName = [appLink.targetApplicationProxy localizedNameForContext:nil]; > + NSString *title = [NSString stringWithFormat:WEB_UI_STRING_KEY("Open in â%@â", "Open in External Application ActionSheet Link", "Title for Open in External Application Link action button"), localizedAppName]; > + action = [[_WKElementAction elementActionWithType:_WKElementActionTypeOpenInExternalApplication customTitle:title] retain]; > + } > + dispatch_semaphore_signal(semaphore); > + }]; > + dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER); > + > + return [action autorelease]; > +} Can we start doing this work at the touch down time so that by the time we need to show a sheet we may already have this information and not need to wait for it?
Comment on attachment 256249 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=256249&action=review >> Source/WebCore/English.lproj/Localizable.strings:413 >> +"Open in External Application ActionSheet Link" = "Open in â%@â"; > > This is not an appropriate key. The key should generally be the same as the value. If there’s ambiguity, the key should be the same as the value plus a disambiguating parenthetical, but in this case there’s no ambiguity. I'll update this. I was following the pattern of all the other ActionSheet links which had this style: /* Title for Copy Link or Image action button */ "Copy ActionSheet Link" = "Copy"; /* Title for Open Link action button */ "Open ActionSheet Link" = "Open"; >> Source/WebKit2/UIProcess/ApplicationStateTracker.h:44 >> + bool hasEntitlement(NSString *entitlement) const; > > Why is this a member function? Laziness. Is there a more appropriate place to put a general hasEntitlement function, or would just a static method on this class be fine?
Created attachment 256653 [details] [PATCH] Proposed Fix Newer version.
Attachment 256653 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:76: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:301: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:303: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:310: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:312: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit2/UIProcess/API/Cocoa/_WKElementAction.mm:95: More than one command on the same line [whitespace/newline] [4] Total errors found: 6 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
https://bugs.webkit.org/show_bug.cgi?id=146658
Wrong link! I meant to paste: r186718 http://trac.webkit.org/changeset/186718