Summary: | Allow sharing from imageSheet on an image document | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Megan Gardner <megan_gardner> | ||||||||
Component: | New Bugs | Assignee: | Megan Gardner <megan_gardner> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, thorton, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Megan Gardner
2019-04-12 18:28:10 PDT
Created attachment 367368 [details]
Patch
Comment on attachment 367368 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367368&action=review > Source/WebKit/UIProcess/API/Cocoa/_WKActivatedElementInfo.mm:113 > +- (instancetype)_initWithType:(_WKActivatedElementType)type imageURL:(NSURL *)imageURL location:(CGPoint)location title:(NSString *)title ID:(NSString *)ID rect:(CGRect)rect image:(WebKit::ShareableBitmap*)image userInfo:(NSDictionary *)userInfo Why a new init method? Why not just add it to the previous one? You could easily have a WKActivatedElementInfo with BOTH a URL and imageURL (<a href="..."><img src="..."></a>), just like InteractionInformation > Source/WebKit/UIProcess/API/Cocoa/_WKElementAction.mm:135 > + if (actionInfo.URL) > + [assistant.delegate actionSheetAssistant:assistant shareElementWithURL:actionInfo.URL rect:actionInfo.boundingRect]; > + else > + [assistant.delegate actionSheetAssistant:assistant shareElementWithURL:actionInfo.imageURL rect:actionInfo.boundingRect]; I would write this as just [assistant.delegate actionSheetAssistant:assistant shareElementWithURL:(actionInfo.URL ?: actionInfo.imageURL) rect:actionInfo.boundingRect]; (maybe without the parens). Also not sure what you want to share if we have both! > Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm:400 > + if (!targetURL) { > + NSURL *imageURL = _positionInformation->imageURL; See the above comments > Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm:544 > + } else if ([elementInfo imageURL]) > + [defaultActions addObject:[_WKElementAction _elementActionWithType:_WKElementActionTypeShare assistant:self]]; instead of an else, you can merge this; if (targetURL) appendOpenActions; if (targetURL || imageURL) appendShareAction Created attachment 367373 [details]
Patch
Created attachment 367464 [details]
Patch
Comment on attachment 367464 [details] Patch Clearing flags on attachment: 367464 Committed r244368: <https://trac.webkit.org/changeset/244368> All reviewed patches have been landed. Closing bug. |