Title: [217818] trunk
Revision
217818
Author
wenson_hs...@apple.com
Date
2017-06-05 21:47:39 -0700 (Mon, 05 Jun 2017)

Log Message

Refactor -[WebItemProviderPasteboard valuesForPasteboardType:inItemSet:] to check readable types
https://bugs.webkit.org/show_bug.cgi?id=172891
<rdar://problem/32204540>

Reviewed by Darin Adler.

Source/WebCore:

Tweaks some logic in PlatformPasteboardIOS and WebItemProviderPasteboard to correctly construct objects in
-valuesForPasteboardType:inItemSet:. Previously, we have a hard-coded map of UTI type to Class in the form of
static helpers titled is{RichText, String, URL, Image, Color}Type in WebItemProviderPasteboard. We would use
these functions to determine whether an NSAttributedString, NSString, NSURL, UIImage or UIColor should be
constructed using the loaded item provider data. This is incorrect for some UTIs, such as public.html, which
cannot actually be used to construct an NSAttributedString -- this caused -valuesForPasteboardType:inItemSet: to
always return nil when attempting to create an object corresponding to public.html.

To fix this, we refactor -valuesForPasteboardType:inItemSet: to instead iterate through UIItemProviderReading-
conformant classes in search for a class that can be created for the given UTI type. If no such class exists, we
then fall back to custom WebKit handling of the dropped UTI type, which so far only includes reading an NSString
with public.plain-text if the UTI is public.html (i.e. reading the HTML source from loaded item provider data).

Covered by 2 new API tests:
DataInteractionTests.ExternalSourceHTMLToContentEditable
DataInteractionTests.ExternalSourceAttributedStringToContentEditable

* platform/ios/PlatformPasteboardIOS.mm:
(WebCore::PlatformPasteboard::stringForType):
(WebCore::PlatformPasteboard::readString):

In some cases, -[WebItemProviderPasteboard valuesForPasteboardType:inItemSet:] may now return an
NSAttributedString when an NSString was previously created. This adjusts for that possibility by returning just
the plain text, -[NSAttributedString string], if the read object was an NSAttributedString.

* platform/ios/WebItemProviderPasteboard.mm:
(allLoadableClasses):
(classForTypeIdentifier):
(-[WebItemProviderPasteboard valuesForPasteboardType:inItemSet:]):

See description above for more details.

(isRichTextType): Deleted.
(isStringType): Deleted.
(isURLType): Deleted.
(isColorType): Deleted.
(isImageType): Deleted.

Removes these heuristics that attempt to "guess" the best UIItemProviderReading class to try and load for a
given UTI type.

* rendering/RenderText.cpp:
(WebCore::RenderText::draggedContentRangesBetweenOffsets):

Adds a nil check to fix a debug assertion hit when running DataInteractionTests.

Tools:

Adds 2 new API tests to exercise data interaction of HTML data and an attributed string into a rich
contenteditable. See WebCore ChangeLog for more details.

* TestWebKitAPI/Tests/ios/DataInteractionTests.mm:
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (217817 => 217818)


--- trunk/Source/WebCore/ChangeLog	2017-06-06 04:00:37 UTC (rev 217817)
+++ trunk/Source/WebCore/ChangeLog	2017-06-06 04:47:39 UTC (rev 217818)
@@ -1,3 +1,57 @@
+2017-06-05  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Refactor -[WebItemProviderPasteboard valuesForPasteboardType:inItemSet:] to check readable types
+        https://bugs.webkit.org/show_bug.cgi?id=172891
+        <rdar://problem/32204540>
+
+        Reviewed by Darin Adler.
+
+        Tweaks some logic in PlatformPasteboardIOS and WebItemProviderPasteboard to correctly construct objects in
+        -valuesForPasteboardType:inItemSet:. Previously, we have a hard-coded map of UTI type to Class in the form of
+        static helpers titled is{RichText, String, URL, Image, Color}Type in WebItemProviderPasteboard. We would use
+        these functions to determine whether an NSAttributedString, NSString, NSURL, UIImage or UIColor should be
+        constructed using the loaded item provider data. This is incorrect for some UTIs, such as public.html, which
+        cannot actually be used to construct an NSAttributedString -- this caused -valuesForPasteboardType:inItemSet: to
+        always return nil when attempting to create an object corresponding to public.html.
+
+        To fix this, we refactor -valuesForPasteboardType:inItemSet: to instead iterate through UIItemProviderReading-
+        conformant classes in search for a class that can be created for the given UTI type. If no such class exists, we
+        then fall back to custom WebKit handling of the dropped UTI type, which so far only includes reading an NSString
+        with public.plain-text if the UTI is public.html (i.e. reading the HTML source from loaded item provider data).
+
+        Covered by 2 new API tests:
+        DataInteractionTests.ExternalSourceHTMLToContentEditable
+        DataInteractionTests.ExternalSourceAttributedStringToContentEditable
+
+        * platform/ios/PlatformPasteboardIOS.mm:
+        (WebCore::PlatformPasteboard::stringForType):
+        (WebCore::PlatformPasteboard::readString):
+
+        In some cases, -[WebItemProviderPasteboard valuesForPasteboardType:inItemSet:] may now return an
+        NSAttributedString when an NSString was previously created. This adjusts for that possibility by returning just
+        the plain text, -[NSAttributedString string], if the read object was an NSAttributedString.
+
+        * platform/ios/WebItemProviderPasteboard.mm:
+        (allLoadableClasses):
+        (classForTypeIdentifier):
+        (-[WebItemProviderPasteboard valuesForPasteboardType:inItemSet:]):
+
+        See description above for more details.
+
+        (isRichTextType): Deleted.
+        (isStringType): Deleted.
+        (isURLType): Deleted.
+        (isColorType): Deleted.
+        (isImageType): Deleted.
+
+        Removes these heuristics that attempt to "guess" the best UIItemProviderReading class to try and load for a
+        given UTI type.
+
+        * rendering/RenderText.cpp:
+        (WebCore::RenderText::draggedContentRangesBetweenOffsets):
+
+        Adds a nil check to fix a debug assertion hit when running DataInteractionTests.
+
 2017-06-05  Dan Bernstein  <m...@apple.com>
 
         Build fix for macOS 10.12

Modified: trunk/Source/WebCore/platform/ios/PlatformPasteboardIOS.mm (217817 => 217818)


--- trunk/Source/WebCore/platform/ios/PlatformPasteboardIOS.mm	2017-06-06 04:00:37 UTC (rev 217817)
+++ trunk/Source/WebCore/platform/ios/PlatformPasteboardIOS.mm	2017-06-06 04:47:39 UTC (rev 217818)
@@ -119,6 +119,9 @@
         if ([value isKindOfClass:[NSURL class]])
             return [(NSURL *)value absoluteString];
 
+        if ([value isKindOfClass:[NSAttributedString class]])
+            return [(NSAttributedString *)value string];
+
         if ([value isKindOfClass:[NSString class]])
             return (NSString *)value;
     }
@@ -392,10 +395,16 @@
 
     id value = [pasteboardItem objectAtIndex:0];
     
-    if (type == String(kUTTypeText) || type == String(kUTTypePlainText)) {
+    if (type == String(kUTTypePlainText) || type == String(kUTTypeHTML)) {
         ASSERT([value isKindOfClass:[NSString class]]);
+        return [value isKindOfClass:[NSString class]] ? value : nil;
+    }
+    if (type == String(kUTTypeText)) {
+        ASSERT([value isKindOfClass:[NSString class]] || [value isKindOfClass:[NSAttributedString class]]);
         if ([value isKindOfClass:[NSString class]])
-            return String(value);
+            return value;
+        if ([value isKindOfClass:[NSAttributedString class]])
+            return [(NSAttributedString *)value string];
     } else if (type == String(kUTTypeURL)) {
         ASSERT([value isKindOfClass:[NSURL class]]);
         if ([value isKindOfClass:[NSURL class]])

Modified: trunk/Source/WebCore/platform/ios/WebItemProviderPasteboard.mm (217817 => 217818)


--- trunk/Source/WebCore/platform/ios/WebItemProviderPasteboard.mm	2017-06-06 04:00:37 UTC (rev 217817)
+++ trunk/Source/WebCore/platform/ios/WebItemProviderPasteboard.mm	2017-06-06 04:47:39 UTC (rev 217818)
@@ -51,34 +51,6 @@
 typedef void(^ItemProviderDataLoadCompletionHandler)(NSData *, NSError *);
 typedef NSDictionary<NSString *, NSURL *> TypeToFileURLMap;
 
-#define MATCHES_UTI_TYPE(type, suffix) [type isEqualToString:(__bridge NSString *)kUTType ## suffix]
-#define MATCHES_UIKIT_TYPE(type, suffix) [type isEqualToString:@"com.apple.uikit. ## suffix ##"]
-
-static BOOL isRichTextType(NSString *type)
-{
-    return MATCHES_UTI_TYPE(type, RTF) || MATCHES_UTI_TYPE(type, RTFD) || MATCHES_UTI_TYPE(type, HTML);
-}
-
-static BOOL isStringType(NSString *type)
-{
-    return MATCHES_UTI_TYPE(type, Text) || MATCHES_UTI_TYPE(type, UTF8PlainText) || MATCHES_UTI_TYPE(type, UTF16PlainText) || MATCHES_UTI_TYPE(type, PlainText);
-}
-
-static BOOL isURLType(NSString *type)
-{
-    return MATCHES_UTI_TYPE(type, URL);
-}
-
-static BOOL isColorType(NSString *type)
-{
-    return MATCHES_UIKIT_TYPE(type, color);
-}
-
-static BOOL isImageType(NSString *type)
-{
-    return MATCHES_UTI_TYPE(type, PNG) || MATCHES_UTI_TYPE(type, JPEG) || MATCHES_UTI_TYPE(type, GIF) || MATCHES_UIKIT_TYPE(type, image);
-}
-
 @interface WebItemProviderRegistrationInfo ()
 {
     RetainPtr<id <UIItemProviderWriting>> _representingObject;
@@ -341,22 +313,28 @@
     return values.autorelease();
 }
 
-static Class classForTypeIdentifier(NSString *typeIdentifier)
+static NSArray<Class<UIItemProviderReading>> *allLoadableClasses()
 {
-    if (isColorType(typeIdentifier))
-        return [getUIColorClass() class];
+    return @[ [getUIColorClass() class], [getUIImageClass() class], [NSURL class], [NSString class], [NSAttributedString class] ];
+}
 
-    if (isImageType(typeIdentifier))
-        return [getUIImageClass() class];
+static Class classForTypeIdentifier(NSString *typeIdentifier, NSString *&outTypeIdentifierToLoad)
+{
+    outTypeIdentifierToLoad = typeIdentifier;
 
-    if (isURLType(typeIdentifier))
-        return [NSURL class];
+    // First, try to load a platform UIItemProviderReading-conformant object as-is.
+    for (Class<UIItemProviderReading> loadableClass in allLoadableClasses()) {
+        if ([[loadableClass readableTypeIdentifiersForItemProvider] containsObject:(NSString *)typeIdentifier])
+            return loadableClass;
+    }
 
-    if (isRichTextType(typeIdentifier))
-        return [NSAttributedString class];
-
-    if (isStringType(typeIdentifier))
+    // If we were unable to load any object, check if the given type identifier is still something
+    // WebKit knows how to handle.
+    if ([typeIdentifier isEqualToString:(NSString *)kUTTypeHTML]) {
+        // Load kUTTypeHTML as a plain text HTML string.
+        outTypeIdentifierToLoad = (NSString *)kUTTypePlainText;
         return [NSString class];
+    }
 
     return nil;
 }
@@ -370,16 +348,17 @@
         if (!provider)
             return;
 
-        Class readableClass = classForTypeIdentifier(pasteboardType);
+        NSString *typeIdentifierToLoad;
+        Class readableClass = classForTypeIdentifier(pasteboardType, typeIdentifierToLoad);
         if (!readableClass)
             return;
 
-        id <UIItemProviderReading> readObject = nil;
-        if (NSData *preloadedData = [retainedSelf _preLoadedDataConformingToType:pasteboardType forItemProviderAtIndex:index])
-            readObject = [[readableClass alloc] initWithItemProviderData:preloadedData typeIdentifier:pasteboardType error:nil];
+        NSData *preloadedData = [retainedSelf _preLoadedDataConformingToType:pasteboardType forItemProviderAtIndex:index];
+        if (!preloadedData)
+            return;
 
-        if (readObject)
-            [values addObject:readObject];
+        if (auto readObject = adoptNS([[readableClass alloc] initWithItemProviderData:preloadedData typeIdentifier:(NSString *)typeIdentifierToLoad error:nil]))
+            [values addObject:readObject.get()];
     }];
 
     return values.autorelease();

Modified: trunk/Source/WebCore/rendering/RenderText.cpp (217817 => 217818)


--- trunk/Source/WebCore/rendering/RenderText.cpp	2017-06-06 04:00:37 UTC (rev 217817)
+++ trunk/Source/WebCore/rendering/RenderText.cpp	2017-06-06 04:47:39 UTC (rev 217818)
@@ -1068,6 +1068,9 @@
 
 Vector<std::pair<unsigned, unsigned>> RenderText::draggedContentRangesBetweenOffsets(unsigned startOffset, unsigned endOffset) const
 {
+    if (!textNode())
+        return { };
+
     auto markers = document().markers().markersFor(textNode(), DocumentMarker::DraggedContent);
     if (markers.isEmpty())
         return { };

Modified: trunk/Tools/ChangeLog (217817 => 217818)


--- trunk/Tools/ChangeLog	2017-06-06 04:00:37 UTC (rev 217817)
+++ trunk/Tools/ChangeLog	2017-06-06 04:47:39 UTC (rev 217818)
@@ -1,3 +1,17 @@
+2017-06-05  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Refactor -[WebItemProviderPasteboard valuesForPasteboardType:inItemSet:] to check readable types
+        https://bugs.webkit.org/show_bug.cgi?id=172891
+        <rdar://problem/32204540>
+
+        Reviewed by Darin Adler.
+
+        Adds 2 new API tests to exercise data interaction of HTML data and an attributed string into a rich
+        contenteditable. See WebCore ChangeLog for more details.
+
+        * TestWebKitAPI/Tests/ios/DataInteractionTests.mm:
+        (TestWebKitAPI::TEST):
+
 2017-06-05  Daniel Bates  <daba...@apple.com>
 
         webkitpy: Abstract Executive class

Modified: trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm (217817 => 217818)


--- trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm	2017-06-06 04:00:37 UTC (rev 217817)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm	2017-06-06 04:47:39 UTC (rev 217818)
@@ -519,6 +519,40 @@
     EXPECT_WK_STREQ("image/jpeg, text/html, text/html", outputValue.UTF8String);
 }
 
+TEST(DataInteractionTests, ExternalSourceHTMLToContentEditable)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
+    [webView synchronouslyLoadTestPageNamed:@"autofocus-contenteditable"];
+    [webView stringByEvaluatingJavaScript:@"getSelection().removeAllRanges()"];
+
+    auto dataInteractionSimulator = adoptNS([[DataInteractionSimulator alloc] initWithWebView:webView.get()]);
+    auto itemProvider = adoptNS([[UIItemProvider alloc] init]);
+    NSData *htmlData = [@"<h1>This is a test</h1>" dataUsingEncoding:NSUTF8StringEncoding];
+    [itemProvider registerDataRepresentationForTypeIdentifier:(NSString *)kUTTypeHTML withData:htmlData];
+    [dataInteractionSimulator setExternalItemProviders:@[ itemProvider.get() ]];
+    [dataInteractionSimulator runFrom:CGPointMake(300, 400) to:CGPointMake(100, 300)];
+
+    NSString *textContent = [webView stringByEvaluatingJavaScript:@"editor.textContent"];
+    EXPECT_WK_STREQ("This is a test", textContent.UTF8String);
+    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"!!editor.querySelector('h1')"].boolValue);
+}
+
+TEST(DataInteractionTests, ExternalSourceAttributedStringToContentEditable)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
+    [webView synchronouslyLoadTestPageNamed:@"autofocus-contenteditable"];
+    [webView stringByEvaluatingJavaScript:@"getSelection().removeAllRanges()"];
+
+    auto dataInteractionSimulator = adoptNS([[DataInteractionSimulator alloc] initWithWebView:webView.get()]);
+    NSDictionary *textAttributes = @{ NSFontAttributeName: [UIFont boldSystemFontOfSize:20] };
+    NSAttributedString *richText = [[NSAttributedString alloc] initWithString:@"This is a test" attributes:textAttributes];
+    auto itemProvider = adoptNS([[UIItemProvider alloc] initWithObject:richText]);
+    [dataInteractionSimulator setExternalItemProviders:@[ itemProvider.get() ]];
+    [dataInteractionSimulator runFrom:CGPointMake(300, 400) to:CGPointMake(100, 300)];
+
+    EXPECT_WK_STREQ("This is a test", [webView stringByEvaluatingJavaScript:@"editor.textContent"].UTF8String);
+}
+
 TEST(DataInteractionTests, ExternalSourceMultipleURLsToContentEditable)
 {
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to