Bug 196891 - Allow sharing from imageSheet on an image document
Summary: Allow sharing from imageSheet on an image document
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Megan Gardner
Keywords: InRadar
Depends on:
Reported: 2019-04-12 18:28 PDT by Megan Gardner
Modified: 2019-04-16 18:42 PDT (History)
3 users (show)

See Also:

Patch (8.44 KB, patch)
2019-04-12 18:36 PDT, Megan Gardner
no flags Details | Formatted Diff | Diff
Patch (16.33 KB, patch)
2019-04-12 19:10 PDT, Megan Gardner
no flags Details | Formatted Diff | Diff
Patch (19.50 KB, patch)
2019-04-15 15:14 PDT, Megan Gardner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Megan Gardner 2019-04-12 18:28:10 PDT
Allow sharing from imageSheet on an image document
Comment 1 Megan Gardner 2019-04-12 18:36:44 PDT
Created attachment 367368 [details]
Comment 2 Tim Horton 2019-04-12 18:41:19 PDT
Comment on attachment 367368 [details]

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
Comment 3 Megan Gardner 2019-04-12 19:10:41 PDT
Created attachment 367373 [details]
Comment 4 Megan Gardner 2019-04-15 15:14:44 PDT
Created attachment 367464 [details]
Comment 5 WebKit Commit Bot 2019-04-16 18:41:06 PDT
Comment on attachment 367464 [details]

Clearing flags on attachment: 367464

Committed r244368: <https://trac.webkit.org/changeset/244368>
Comment 6 WebKit Commit Bot 2019-04-16 18:41:07 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2019-04-16 18:42:21 PDT