WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
171154
Support writing link titles to the pasteboard when beginning data interaction on a link
https://bugs.webkit.org/show_bug.cgi?id=171154
Summary
Support writing link titles to the pasteboard when beginning data interaction...
Wenson Hsieh
Reported
2017-04-21 18:19:48 PDT
Work towards <
rdar://problem/31356937
>
Attachments
Part (1/3)
(24.29 KB, patch)
2017-04-21 18:48 PDT
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
Attempt to fix 32-bit iOS.
(23.77 KB, patch)
2017-04-21 20:16 PDT
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
Really fix iOS build
(23.81 KB, patch)
2017-04-21 20:41 PDT
,
Wenson Hsieh
aestes
: review+
Details
Formatted Diff
Diff
Patch for landing
(24.15 KB, patch)
2017-04-21 22:37 PDT
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Wenson Hsieh
Comment 1
2017-04-21 18:48:08 PDT
Created
attachment 307860
[details]
Part (1/3)
Wenson Hsieh
Comment 2
2017-04-21 20:16:09 PDT
Created
attachment 307869
[details]
Attempt to fix 32-bit iOS.
Wenson Hsieh
Comment 3
2017-04-21 20:41:01 PDT
Created
attachment 307874
[details]
Really fix iOS build
Andy Estes
Comment 4
2017-04-21 21:43:29 PDT
Comment on
attachment 307874
[details]
Really fix iOS build View in context:
https://bugs.webkit.org/attachment.cgi?id=307874&action=review
> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:42 > +#if ENABLE(DATA_INTERACTION) > +#import <UIKit/NSURL+UIItemProvider.h> > +#endif
This is fine for now, but the build will break once we start building against the Public iOS 11 SDK because NSURL+UIItemProvider.h is a Private header. We'll need to make a SPI header for this.
> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:296 > + RetainPtr<NSMutableArray> objectRepresentations = adoptNS([[NSMutableArray alloc] init]);
This is one of the cases where I prefer auto, since the type is clear from the right hand side.
> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:299 > + NSURL *nsURL = (NSURL *)url.url; > + if (nsURL) {
These two lines could be: if (NSURL *nsURL = (NSURL *)url.url) {
> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:307 > + [m_pasteboard setItemsFromObjectRepresentations:@[[WebPasteboardItemData itemWithRepresentingObjects:objectRepresentations.get() additionalData:@{ }]]];
Can you use nil for the additionalData argument, or does it really need to be an empty NSDictionary?
> Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:176 > + [itemProvider registerDataRepresentationForTypeIdentifier:typeIdentifier visibility:UIItemProviderRepresentationOptionsVisibilityAll loadHandler:^NSProgress *(void (^completionHandler)(NSData *, NSError *))
Can you use the UIItemProviderDataLoadHandler typedef here?
Wenson Hsieh
Comment 5
2017-04-21 22:28:55 PDT
Comment on
attachment 307874
[details]
Really fix iOS build View in context:
https://bugs.webkit.org/attachment.cgi?id=307874&action=review
Thanks for taking a look!
>> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:42 >> +#endif > > This is fine for now, but the build will break once we start building against the Public iOS 11 SDK because NSURL+UIItemProvider.h is a Private header. We'll need to make a SPI header for this.
Good point. I think the intention is to make NSURL+UIItemProvider.h public at some point (I suppose that begs the question, where will _title be defined :|). But if PoR is for this to remain private, I'll roll this into UIKitSPI.h, defining _title in a private category on NSURL for non-internal SDKs.
>> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:296 >> + RetainPtr<NSMutableArray> objectRepresentations = adoptNS([[NSMutableArray alloc] init]); > > This is one of the cases where I prefer auto, since the type is clear from the right hand side.
Yep! Changed to use auto.
>> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:299 >> + if (nsURL) { > > These two lines could be: > > if (NSURL *nsURL = (NSURL *)url.url) {
True! (I can also drop the (NSURL *) as well, since the conversion is implicit). Fixed.
>> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:307 >> + [m_pasteboard setItemsFromObjectRepresentations:@[[WebPasteboardItemData itemWithRepresentingObjects:objectRepresentations.get() additionalData:@{ }]]]; > > Can you use nil for the additionalData argument, or does it really need to be an empty NSDictionary?
Good point. I could use nil here, but I'm actually going to refactor this in the next patch (not up yet :p) so that we don't have to use initialize WebPasteboardItemDatas in this way at all. I'll change it to nil here though.
>> Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:176 >> + [itemProvider registerDataRepresentationForTypeIdentifier:typeIdentifier visibility:UIItemProviderRepresentationOptionsVisibilityAll loadHandler:^NSProgress *(void (^completionHandler)(NSData *, NSError *)) > > Can you use the UIItemProviderDataLoadHandler typedef here?
I wanted to avoid using that, since it's marked as deprecated in the UIKit headers (UIItemProvider_Private.h). I agree it looks much cleaner using a typedef though -- I'll declare a typedef at the top of this file and use that here instead.
Wenson Hsieh
Comment 6
2017-04-21 22:37:37 PDT
Created
attachment 307887
[details]
Patch for landing
WebKit Commit Bot
Comment 7
2017-04-21 23:18:56 PDT
Comment on
attachment 307887
[details]
Patch for landing Clearing flags on attachment: 307887 Committed
r215663
: <
http://trac.webkit.org/changeset/215663
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug