Tapping and holding a link should have a share option rdar://problem/21319702
Created attachment 264347 [details] Patch
Attachment 264347 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/Cocoa/_WKElementAction.mm:132: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 264406 [details] Patch
Attachment 264406 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/Cocoa/_WKElementAction.mm:132: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 264406 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264406&action=review > Source/WebKit2/UIProcess/API/Cocoa/_WKElementAction.mm:133 > + NSLog(@"actionInfo._interactionLocation for share item is %@", NSStringFromPoint(actionInfo._interactionLocation)); Please no. > Source/WebKit2/UIProcess/ios/WKPDFView.mm:726 > + if ([_webSelectionAssistant respondsToSelector:@selector(showShareSheetFor:fromRect:)]) Wonder if we still need all these respondsToSelector checks these days. > Source/WebKit2/UIProcess/ios/WKPDFView.mm:727 > + [_webSelectionAssistant showShareSheetFor:[url absoluteString] fromRect:boundingRect]; url.absoluteString (and above too)
Comment on attachment 264406 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264406&action=review >> Source/WebKit2/UIProcess/ios/WKPDFView.mm:726 >> + if ([_webSelectionAssistant respondsToSelector:@selector(showShareSheetFor:fromRect:)]) > > Wonder if we still need all these respondsToSelector checks these days. I don't think we do need the responds to selector here.
(In reply to comment #6) > Comment on attachment 264406 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=264406&action=review > > >> Source/WebKit2/UIProcess/ios/WKPDFView.mm:726 > >> + if ([_webSelectionAssistant respondsToSelector:@selector(showShareSheetFor:fromRect:)]) > > > > Wonder if we still need all these respondsToSelector checks these days. > > I don't think we do need the responds to selector here. Thank you Enrica and Tim! Checked in http://trac.webkit.org/changeset/191805
Comment on attachment 264406 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264406&action=review > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3388 > + [_textSelectionAssistant showShareSheetFor:[url absoluteString] fromRect:boundingRect]; > + else if (_webSelectionAssistant && [_webSelectionAssistant respondsToSelector:@selector(showShareSheetFor:fromRect:)]) > + [_webSelectionAssistant showShareSheetFor:[url absoluteString] fromRect:boundingRect]; Do these strings get interpreted as URLs by some code on the other side of this API, or do they end up being presented to the user? If it’s the latter, then we shouldn’t use -absoluteString, because it doesn’t give a good representation of certain URLs, such as ones with non-ASCII characters in the host or the path. Instead, -_web_userVisibleString should work. If these strings do get turned back into URLs, and we can’t use an alternative API that takes NSURLs, then we should consider using -_web_originalDataAsString here and the reverse transformation on the receiving end. >> Source/WebKit2/UIProcess/ios/WKPDFView.mm:727 >> + [_webSelectionAssistant showShareSheetFor:[url absoluteString] fromRect:boundingRect]; > > url.absoluteString (and above too) Same comment here.
(In reply to comment #8) > Comment on attachment 264406 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=264406&action=review > > > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3388 > > + [_textSelectionAssistant showShareSheetFor:[url absoluteString] fromRect:boundingRect]; > > + else if (_webSelectionAssistant && [_webSelectionAssistant respondsToSelector:@selector(showShareSheetFor:fromRect:)]) > > + [_webSelectionAssistant showShareSheetFor:[url absoluteString] fromRect:boundingRect]; > > Do these strings get interpreted as URLs by some code on the other side of > this API, or do they end up being presented to the user? If it’s the latter, > then we shouldn’t use -absoluteString, because it doesn’t give a good > representation of certain URLs, such as ones with non-ASCII characters in > the host or the path. Instead, -_web_userVisibleString should work. If these > strings do get turned back into URLs, and we can’t use an alternative API > that takes NSURLs, then we should consider using -_web_originalDataAsString > here and the reverse transformation on the receiving end. > > >> Source/WebKit2/UIProcess/ios/WKPDFView.mm:727 > >> + [_webSelectionAssistant showShareSheetFor:[url absoluteString] fromRect:boundingRect]; > > > > url.absoluteString (and above too) > > Same comment here. Thank you Dan! This is presented to the user, and I was able to see the bug you referred to with non-ascii URLs. I will attach a follow-up patch.
Created attachment 264597 [details] Follow-up fix for non-ascii URLs
Thanks, Dan! http://trac.webkit.org/changeset/191894