Enable copy paste on iOS apps for Mac
Created attachment 343947 [details] Patch
<rdar://problem/41451148>
Comment on attachment 343947 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343947&action=review > Source/WebCore/platform/ios/WebItemProviderPasteboard.h:30 > +#if ENABLE(MINIMAL_SIMULATOR) Since this is a private header, this should be TARGET_OS_IOSMAC instead. > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:29 > +#if ENABLE(DATA_INTERACTION) || ENABLE(MINIMAL_SIMULATOR) Nit - let's update the comment after the #endif as well!
Created attachment 343969 [details] Patch
Created attachment 343986 [details] Patch
Comment on attachment 343986 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343986&action=review > Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:262 > + [pasteboard setItems:@[itemDictionary.get()]]; Code below uses a style with spaces between the @[ and the ], and it would be nice to be consistent. I’m also surprised that we want an array with a single item that is a dictionary here, but I’m willing to believe it! > Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:366 > +#if !ENABLE(MINIMAL_SIMULATOR) > nsURL._title = pasteboardURL.title.isEmpty() ? userVisibleString(pasteboardURL.url) : (NSString *)pasteboardURL.title; > +#endif I think we need a comment about why we don’t do that on iOS running on Mac. It seems random and arbitrary without a comment, although arguably more terse and elegant. If the #if was something more along the rough lines of NSURL_SUPPORTS_TITLE, then it would be self explanatory; the comment would go where that is defined instead. I think that would be best. Following a pattern like PASTEBOARD_SUPPORTS_ITEM_PROVIDERS. > Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:399 > +#if !ENABLE(MINIMAL_SIMULATOR) > if (!url.title.isEmpty()) > nsURL._title = url.title; > +#endif Ditto. > Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:641 > +#if PASTEBOARD_SUPPORTS_ITEM_PROVIDERS && !ENABLE(MINIMAL_SIMULATOR) > title = [url _title]; Can we really do without the title? I understand that we can’t use the "_title" property. I am concerned because we may need the title for things like good quality rich links. > Source/WebCore/platform/ios/WebItemProviderPasteboard.h:35 > +#define UIItemProvider NSItemProvider > +#define UIItemProviderReading NSItemProviderReading > +#define UIItemProviderWriting NSItemProviderWriting > +#define UIItemProviderRepresentationOptionsVisibilityAll NSItemProviderRepresentationVisibilityAll Is this the kind of hack we want to do to share code across the platforms? I don’t see this sort of thing elsewhere, but maybe we decided on this the past? > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:29 > -#if ENABLE(DATA_INTERACTION) > +#if ENABLE(DATA_INTERACTION) || ENABLE(MINIMAL_SIMULATOR) This combination of feature conditional plus platform conditional seems particularly inelegant. Is there a way to do better? > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:289 > +#if !ENABLE(MINIMAL_SIMULATOR) > + [itemProvider setPreferredPresentationSize:self.preferredPresentationSize]; > [itemProvider setPreferredPresentationStyle:uiPreferredPresentationStyle(self.preferredPresentationStyle)]; > [itemProvider setTeamData:self.teamData]; > +#endif Why? Comment please.
You may be feeling that "a few things have to be off just for now, so I’ll just turn them off knowing I’m coming back here really soon”, but my past experience is that in practice there is lots of code like this many, many years later, so the comments may well be quite valuable. And if not, you can delete them.
Comment on attachment 343986 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343986&action=review > Source/WebCore/platform/ios/WebItemProviderPasteboard.h:36 > + This is temporary until we switch to NSItemProviders everywhere, which will hopefully be soon.
Created attachment 344134 [details] Patch for landing
Comment on attachment 344134 [details] Patch for landing Clearing flags on attachment: 344134 Committed r233436: <https://trac.webkit.org/changeset/233436>
All reviewed patches have been landed. Closing bug.