Changeset 43850 in webkit

Timestamp:

May 18, 2009 9:17:26 PM (15 years ago)

Author:

eric@webkit.org

Message:

2009-05-17 Eric Seidel <​eric@webkit.org>

Reviewed by Darin Adler.

ClipboardMac cleanup
​https://bugs.webkit.org/show_bug.cgi?id=25847

I'm mostly just moving code, however there are 3 changes I made while moving, detailed below.

No functional changes, so no tests.

• platform/mac/ClipboardMac.mm: (WebCore::ClipboardMac::clearData): (WebCore::absoluteURLsFromPasteboardFilenames): Broke out logic for filenames into its own function to make the caller more readable. (WebCore::absoluteURLsFromPasteboard): Broke out logic from getData into absoluteURLsFromPasteboard. This returns an NSArray so that we can use -[NSArray componentsJoinedByString] in the caller (which is cleaner than the manual "\n" addition before). This also access to the full list of file urls for future callers. (WebCore::ClipboardMac::getData): unsigned count = (type == "URL") ? 1 : [fileList count]; is now an explicit check for "URL", before it was a check for != "text/uri-list" which was much more confusing in my opinion. text/uri-list and URL are the only two types which map to NSURLPboardType in cocoaTypeFromMIMEType(). (WebCore::ClipboardMac::types): I removed an extra if (!types) check, right before [types count]. In Obj-C messaging nil will return 0 (size of a pointer), so it's safe to message nil here and expect it to return 0.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/mac/ClipboardMac.mm (modified) (4 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-05-17  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        ClipboardMac cleanup
+        https://bugs.webkit.org/show_bug.cgi?id=25847
+
+        I'm mostly just moving code, however there are 3 changes I made while moving, detailed below.
+
+        No functional changes, so no tests.
+
+        * platform/mac/ClipboardMac.mm:
+        (WebCore::ClipboardMac::clearData):
+        (WebCore::absoluteURLsFromPasteboardFilenames):
+          Broke out logic for filenames into its own function to make the caller more readable.
+        (WebCore::absoluteURLsFromPasteboard):
+          Broke out logic from getData into absoluteURLsFromPasteboard. This returns an NSArray
+          so that we can use -[NSArray componentsJoinedByString] in the caller (which is
+          cleaner than the manual "\n" addition before).
+          This also access to the full list of file urls for future callers.
+        (WebCore::ClipboardMac::getData):
+          unsigned count = (type == "URL") ? 1 : [fileList count]; is now an
+          explicit check for "URL", before it was a check for != "text/uri-list" which
+          was much more confusing in my opinion.  text/uri-list and URL are the only
+          two types which map to NSURLPboardType in cocoaTypeFromMIMEType().
+        (WebCore::ClipboardMac::types):
+          I removed an extra if (!types) check, right before [types count].  In Obj-C
+          messaging nil will return 0 (size of a pointer), so it's safe to message nil
+          here and expect it to return 0.
+
 2009-05-18  David Levin  <levin@chromium.org>
 

trunk/WebCore/platform/mac/ClipboardMac.mm

@@ -122 +122 @@
 
     NSString *cocoaType = cocoaTypeFromMIMEType(type);
-    if (cocoaType) {
+    if (cocoaType)
         [m_pasteboard.get() setString:@"" forType:cocoaType];
-    }
 }
 
@@ -135 +134 @@
 
     [m_pasteboard.get() declareTypes:[NSArray array] owner:nil];
+}
+
+static NSArray *absoluteURLsFromPasteboardFilenames(NSPasteboard* pasteboard, bool onlyFirstURL)
+{
+    NSArray *fileList = [pasteboard propertyListForType:NSFilenamesPboardType];
+
+    // FIXME: Why does this code need to guard against bad values on the pasteboard?
+    ASSERT(!fileList || [fileList isKindOfClass:[NSArray class]]);
+    if (!fileList || ![fileList isKindOfClass:[NSArray class]] || ![fileList count])
+        return nil;
+
+    NSUInteger count = onlyFirstURL ? 1 : [fileList count];
+    NSMutableArray *urls = [NSMutableArray array];
+    for (NSUInteger i = 0; i < count; i++) {
+        NSString *string = [fileList objectAtIndex:i];
+
+        ASSERT([string isKindOfClass:[NSString class]]);  // Added to understand why this if code is here
+        if (![string isKindOfClass:[NSString class]])
+            return nil; // Non-string object in the list, bail out!  FIXME: When can this happen?
+
+        NSURL *url = [NSURL fileURLWithPath:string];
+        [urls addObject:[url absoluteString]];
+    }
+    return urls;
+}
+
+static NSArray *absoluteURLsFromPasteboard(NSPasteboard* pasteboard, bool onlyFirstURL)
+{
+    // NOTE: We must always check [availableTypes containsObject:] before accessing pasteboard data
+    // or CoreFoundation will printf when there is not data of the corresponding type.
+    NSArray *availableTypes = [pasteboard types];
+
+    // Try NSFilenamesPboardType because it contains a list
+    if ([availableTypes containsObject:NSFilenamesPboardType]) {
+        if (NSArray* absoluteURLs = absoluteURLsFromPasteboardFilenames(pasteboard, onlyFirstURL))
+            return absoluteURLs;
+    }
+
+    // Fallback to NSURLPboardType (which is a single URL)
+    if ([availableTypes containsObject:NSURLPboardType]) {
+        if (NSURL *url = [NSURL URLFromPasteboard:pasteboard])
+            return [NSArray arrayWithObject:[url absoluteString]];
+    }
+
+    // No file paths on the pasteboard, return nil
+    return nil;
 }
 
@@ -142 +187 @@
     if (policy() != ClipboardReadable)
         return String();
-    
+
     NSString *cocoaType = cocoaTypeFromMIMEType(type);
     NSString *cocoaValue = nil;
-    NSArray *availableTypes = [m_pasteboard.get() types];
-
-    // Fetch the data in different ways for the different Cocoa types
-
+
+    // Grab the value off the pasteboard corresponding to the cocoaType
     if ([cocoaType isEqualToString:NSURLPboardType]) {
-        // When both URL and filenames are present, filenames is superior since it can contain a list.
-        // must check this or we get a printf from CF when there's no data of this type
-        if ([availableTypes containsObject:NSFilenamesPboardType]) {
-            NSArray *fileList = [m_pasteboard.get() propertyListForType:NSFilenamesPboardType];
-            if (fileList && [fileList isKindOfClass:[NSArray class]]) {
-                unsigned count = [fileList count];
-                if (count > 0) {
-                    if (type != "text/uri-list")
-                        count = 1;
-                    NSMutableString *urls = [NSMutableString string];
-                    unsigned i;
-                    for (i = 0; i < count; i++) {
-                        if (i > 0) {
-                            [urls appendString:@"\n"];
-                        }
-                        NSString *string = [fileList objectAtIndex:i];
-                        if (![string isKindOfClass:[NSString class]])
-                            break;
-                        NSURL *url = [NSURL fileURLWithPath:string];
-                        [urls appendString:[url absoluteString]];
-                    }
-                    if (i == count)
-                        cocoaValue = urls;
-                }
-            }
-        }
-        if (!cocoaValue) {
-            // must check this or we get a printf from CF when there's no data of this type
-            if ([availableTypes containsObject:NSURLPboardType]) {
-                NSURL *url = [NSURL URLFromPasteboard:m_pasteboard.get()];
-                if (url) {
-                    cocoaValue = [url absoluteString];
-                }
-            }
-        }
+        // "URL" and "text/url-list" both map to NSURLPboardType in cocoaTypeFromMIMEType(), "URL" only wants the first URL
+        bool onlyFirstURL = (type == "URL");
+        NSArray *absoluteURLs = absoluteURLsFromPasteboard(m_pasteboard.get(), onlyFirstURL);
+        cocoaValue = [absoluteURLs componentsJoinedByString:@"\n"];
     } else if ([cocoaType isEqualToString:NSStringPboardType]) {
         cocoaValue = [[m_pasteboard.get() stringForType:cocoaType] precomposedStringWithCanonicalMapping];
-    } else if (cocoaType) {        
+    } else if (cocoaType)
         cocoaValue = [m_pasteboard.get() stringForType:cocoaType];
-    }
 
     // Enforce changeCount ourselves for security.  We check after reading instead of before to be
@@ -247 +258 @@
 
     HashSet<String> result;
-    if (types) {
-        unsigned count = [types count];
-        unsigned i;
-        for (i = 0; i < count; i++) {
-            NSString *pbType = [types objectAtIndex:i];
-            if ([pbType isEqualToString:@"NeXT plain ascii pasteboard type"])
-                continue;   // skip this ancient type that gets auto-supplied by some system conversion
-
-            String str = MIMETypeFromCocoaType(pbType);
-            if (!result.contains(str))
-                result.add(str);
-        }
+    NSUInteger count = [types count];
+    for (NSUInteger i = 0; i < count; i++) {
+        NSString *pbType = [types objectAtIndex:i];
+        if ([pbType isEqualToString:@"NeXT plain ascii pasteboard type"])
+            continue;   // skip this ancient type that gets auto-supplied by some system conversion
+
+        String str = MIMETypeFromCocoaType(pbType);
+        if (!result.contains(str))
+            result.add(str);
     }
     return result;