Changeset 262047 in webkit


Ignore:
Timestamp:
May 21, 2020 9:42:47 PM (4 years ago)
Author:
Wenson Hsieh
Message:

DataTransfer.files contains multiple files when pasting a single image with multiple representations
​https://bugs.webkit.org/show_bug.cgi?id=212245
<rdar://problem/60240436>

Reviewed by Tim Horton.

Source/WebCore:

When pasting or dropping a single image that is backed by multiple representations in NSPasteboard (or
UIPasteboard), we currently report more than one File to the page via DataTransfer.files. This is because
Pasteboard::read(PasteboardFileReader&), which is responsible for converting the contents of the pasteboard
into a list of files, currently iterates over every pasteboard type and adds each of them as a file. This is
wrong when an item has multiple type representations.

To differentiate the case where a single item has multiple representations from the case where it has multiple
pasteboard items, we use allPasteboardItemInfo() instead to grab a per-item list of types from the pasteboard
on Cocoa platforms, and only create at most 1 file per item using the highest fidelity type that contains data.

Test: PasteImage.PasteImageWithMultipleRepresentations

  • platform/cocoa/PasteboardCocoa.mm:

(WebCore::Pasteboard::read):

Tools:

  • DumpRenderTree/mac/DumpRenderTreePasteboard.mm:

(-[LocalPasteboard _clearContentsWithoutUpdatingChangeCount]):
(-[LocalPasteboard _addTypesWithoutUpdatingChangeCount:owner:]):
(-[LocalPasteboard writeObjects:]):
(-[LocalPasteboard pasteboardItems]):

Adjust DumpRenderTree's LocalPasteboard so that it lazily populates the pasteboard when constructing
NSPasteboardItems. To do this, we need to make a few adjustments:

  1. When reifying NSPasteboardItems from LocalPasteboard, ask the owner (WebHTMLView) to provide pasteboard

data for each pasteboard type that was promised by WebKit, but was not eagerly written to the pasteboard.

  1. Cache pasteboard items that were created, so that we don't repeatedly ask WebHTMLView to provide

pasteboard data. WebHTMLView doesn't currently support this, and suffers from a bug where TIFF data may
only be provided once. This was fixed for WebKit2, but not for WebKit1.

  1. Maintain a separate hash list of original pasteboard types (which may not be UTIs) that were handed to

LocalPasteboard by WebKit. We use these original types in step (1).

  • TestWebKitAPI/Tests/WebKitCocoa/PasteImage.mm:

Add a new API test to verify that one file is exposed via the DataTranfer when the pasteboard contains a single
image with two image representations, but two files are exposed when the pasteboard contains two images, each
with a single representation.

(writeImageDataToPasteboard):

Overload this helper method with two additional variants: one that takes a dictionary of pasteboard types to
data, and another that takes an array of dictionaries, each representing a single item's types and data.

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r262046 r262047  
     12020-05-21  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        DataTransfer.files contains multiple files when pasting a single image with multiple representations
     4        https://bugs.webkit.org/show_bug.cgi?id=212245
     5        <rdar://problem/60240436>
     6
     7        Reviewed by Tim Horton.
     8
     9        When pasting or dropping a single image that is backed by multiple representations in NSPasteboard (or
     10        UIPasteboard), we currently report more than one `File` to the page via `DataTransfer.files`. This is because
     11        `Pasteboard::read(PasteboardFileReader&)`, which is responsible for converting the contents of the pasteboard
     12        into a list of files, currently iterates over every pasteboard type and adds each of them as a file. This is
     13        wrong when an item has multiple type representations.
     14
     15        To differentiate the case where a single item has multiple representations from the case where it has multiple
     16        pasteboard items, we use `allPasteboardItemInfo()` instead to grab a per-item list of types from the pasteboard
     17        on Cocoa platforms, and only create at most 1 file per item using the highest fidelity type that contains data.
     18
     19        Test: PasteImage.PasteImageWithMultipleRepresentations
     20
     21        * platform/cocoa/PasteboardCocoa.mm:
     22        (WebCore::Pasteboard::read):
     23
    1242020-05-21  Simon Fraser  <simon.fraser@apple.com>
    225

  • trunk/Source/WebCore/platform/cocoa/PasteboardCocoa.mm

    r260366 r262047  
    225225    }
    226226
    227     auto cocoaTypes = readTypesWithSecurityCheck();
    228     HashSet<const char*> existingMIMEs;
    229     for (auto& cocoaType : cocoaTypes) {
    230         auto imageType = cocoaTypeToImageType(cocoaType);
    231         const char* mimeType = imageTypeToMIMEType(imageType);
    232         if (!mimeType)
    233             continue;
    234         if (existingMIMEs.contains(mimeType))
    235             continue;
    236         auto buffer = readBufferForTypeWithSecurityCheck(cocoaType);
    237 #if PLATFORM(MAC)
    238         if (buffer && imageType == ImageType::TIFF)
    239             buffer = convertTIFFToPNG(buffer.releaseNonNull());
    240 #endif
    241         if (!buffer)
    242             continue;
    243         existingMIMEs.add(mimeType);
    244         reader.readBuffer(imageTypeToFakeFilename(imageType), mimeType, buffer.releaseNonNull());
     227    auto allInfo = allPasteboardItemInfo();
     228    if (!allInfo)
     229        return;
     230
     231    for (size_t itemIndex = 0; itemIndex < allInfo->size(); ++itemIndex) {
     232        auto& info = allInfo->at(itemIndex);
     233        for (auto cocoaType : info.platformTypesByFidelity) {
     234            auto imageType = cocoaTypeToImageType(cocoaType);
     235            auto* mimeType = imageTypeToMIMEType(imageType);
     236            if (!mimeType)
     237                continue;
     238            auto buffer = readBuffer(itemIndex, cocoaType);
     239#if PLATFORM(MAC)
     240            if (buffer && imageType == ImageType::TIFF)
     241                buffer = convertTIFFToPNG(buffer.releaseNonNull());
     242#endif
     243            if (buffer) {
     244                reader.readBuffer(imageTypeToFakeFilename(imageType), mimeType, buffer.releaseNonNull());
     245                break;
     246            }
     247        }
    245248    }
    246249}

  • trunk/Tools/ChangeLog

    r262040 r262047  
     12020-05-21  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        DataTransfer.files contains multiple files when pasting a single image with multiple representations
     4        https://bugs.webkit.org/show_bug.cgi?id=212245
     5        <rdar://problem/60240436>
     6
     7        Reviewed by Tim Horton.
     8
     9        * DumpRenderTree/mac/DumpRenderTreePasteboard.mm:
     10        (-[LocalPasteboard _clearContentsWithoutUpdatingChangeCount]):
     11        (-[LocalPasteboard _addTypesWithoutUpdatingChangeCount:owner:]):
     12        (-[LocalPasteboard writeObjects:]):
     13        (-[LocalPasteboard pasteboardItems]):
     14
     15        Adjust DumpRenderTree's LocalPasteboard so that it lazily populates the pasteboard when constructing
     16        NSPasteboardItems. To do this, we need to make a few adjustments:
     17
     18        1.      When reifying NSPasteboardItems from LocalPasteboard, ask the owner (WebHTMLView) to provide pasteboard
     19                data for each pasteboard type that was promised by WebKit, but was not eagerly written to the pasteboard.
     20
     21        2.      Cache pasteboard items that were created, so that we don't repeatedly ask WebHTMLView to provide
     22                pasteboard data. WebHTMLView doesn't currently support this, and suffers from a bug where TIFF data may
     23                only be provided once. This was fixed for WebKit2, but not for WebKit1.
     24
     25        3.      Maintain a separate hash list of original pasteboard types (which may not be UTIs) that were handed to
     26                LocalPasteboard by WebKit. We use these original types in step (1).
     27
     28        * TestWebKitAPI/Tests/WebKitCocoa/PasteImage.mm:
     29
     30        Add a new API test to verify that one file is exposed via the DataTranfer when the pasteboard contains a single
     31        image with two image representations, but two files are exposed when the pasteboard contains two images, each
     32        with a single representation.
     33
     34        (writeImageDataToPasteboard):
     35
     36        Overload this helper method with two additional variants: one that takes a dictionary of pasteboard types to
     37        data, and another that takes an array of dictionaries, each representing a single item's types and data.
     38
    1392020-05-21  Robin Morisset  <rmorisset@apple.com>
    240

  • trunk/Tools/DumpRenderTree/mac/DumpRenderTreePasteboard.mm

    r259843 r262047  
    4646    RetainPtr<id> _owner;
    4747    RetainPtr<NSString> _pasteboardName;
    48     RetainPtr<NSMutableArray<NSPasteboardItem *>> _writtenPasteboardItems;
     48    RetainPtr<NSArray<NSPasteboardItem *>> _cachedPasteboardItems;
    4949    NSInteger _changeCount;
    5050
    5151    ListHashSet<RetainPtr<CFStringRef>, WTF::RetainPtrObjectHash<CFStringRef>> _types;
     52    ListHashSet<RetainPtr<CFStringRef>, WTF::RetainPtrObjectHash<CFStringRef>> _originalTypes;
    5253    HashMap<RetainPtr<CFStringRef>, RetainPtr<CFDataRef>, WTF::RetainPtrObjectHash<CFStringRef>, WTF::RetainPtrObjectHashTraits<CFStringRef>> _data;
    5354}
    … …  
    145146- (void)_clearContentsWithoutUpdatingChangeCount
    146147{
    147     _writtenPasteboardItems = nil;
     148    _cachedPasteboardItems = nil;
    148149    _types.clear();
     150    _originalTypes.clear();
    149151    _data.clear();
    150152}
    … …  
    169171    _owner = newOwner;
    170172
    171     for (NSString *type in newTypes)
     173    for (NSString *type in newTypes) {
    172174        _types.add(toUTI(type));
     175        _originalTypes.add((__bridge CFStringRef)type);
     176    }
    173177}
    174178
    … …  
    240244- (BOOL)writeObjects:(NSArray<id <NSPasteboardWriting>> *)objects
    241245{
    242     _writtenPasteboardItems = adoptNS([[NSMutableArray<NSPasteboardItem *> alloc] initWithCapacity:objects.count]);
     246    auto items = adoptNS([[NSMutableArray<NSPasteboardItem *> alloc] initWithCapacity:objects.count]);
    243247    for (id <NSPasteboardWriting> object in objects) {
    244248        ASSERT([object isKindOfClass:NSPasteboardItem.class]);
    245         [_writtenPasteboardItems addObject:(NSPasteboardItem *)object];
     249        [items addObject:(NSPasteboardItem *)object];
    246250        for (NSString *type in [object writableTypesForPasteboard:self]) {
    247251            [self addTypes:@[ type ] owner:self];
    … …  
    255259    }
    256260
     261    _cachedPasteboardItems = items.get();
    257262    return YES;
    258263}
    … …  
    260265- (NSArray<NSPasteboardItem *> *)pasteboardItems
    261266{
    262     if (_writtenPasteboardItems)
    263         return _writtenPasteboardItems.get();
     267    if (_cachedPasteboardItems)
     268        return _cachedPasteboardItems.get();
    264269
    265270    auto item = adoptNS([[NSPasteboardItem alloc] init]);
     271    for (auto& type : _originalTypes) {
     272        if (!_data.contains(toUTI((__bridge NSString *)type.get())))
     273            [_owner pasteboard:self provideDataForType:(__bridge NSString *)type.get()];
     274    }
     275
    266276    for (const auto& typeAndData : _data) {
    267277        NSData *data = (__bridge NSData *)typeAndData.value.get();
    … …  
    269279        [item setData:data forType:[NSPasteboard _modernPasteboardType:type]];
    270280    }
    271     return @[ item.get() ];
     281
     282    _cachedPasteboardItems = @[ item.get() ];
     283    return _cachedPasteboardItems.get();
    272284}
    273285

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteImage.mm

    r260366 r262047  
    3838
    3939#if PLATFORM(MAC)
     40
    4041void writeImageDataToPasteboard(NSString *type, NSData *data)
    4142{
    42     [[NSPasteboard generalPasteboard] declareTypes:@[type] owner:nil];
    43     [[NSPasteboard generalPasteboard] setData:data forType:type];
    44 }
     43    [NSPasteboard.generalPasteboard declareTypes:@[type] owner:nil];
     44    [NSPasteboard.generalPasteboard setData:data forType:type];
     45}
     46
     47void writeImageDataToPasteboard(NSDictionary <NSString *, NSData *> *typesAndData)
     48{
     49    [NSPasteboard.generalPasteboard declareTypes:typesAndData.allKeys owner:nil];
     50    for (NSString *type in typesAndData)
     51        [NSPasteboard.generalPasteboard setData:typesAndData[type] forType:type];
     52}
     53
     54void writeImageDataToPasteboard(NSArray<NSDictionary <NSString *, NSData *> *> *items)
     55{
     56    [NSPasteboard.generalPasteboard clearContents];
     57    auto pasteboardItems = adoptNS([[NSMutableArray alloc] initWithCapacity:items.count]);
     58    for (NSDictionary<NSString *, NSData *> *typesAndData in items) {
     59        auto pasteboardItem = adoptNS([[NSPasteboardItem alloc] init]);
     60        for (NSString *type in typesAndData)
     61            [pasteboardItem setData:typesAndData[type] forType:type];
     62        [pasteboardItems addObject:pasteboardItem.get()];
     63    }
     64    [NSPasteboard.generalPasteboard writeObjects:pasteboardItems.get()];
     65}
     66
    4567#else
     68
    4669void writeImageDataToPasteboard(NSString *type, NSData *data)
    4770{
    48     [[UIPasteboard generalPasteboard] setItems:@[@{ type: data }]];
    49 }
     71    UIPasteboard.generalPasteboard.items = @[ @{ type: data } ];
     72}
     73
     74void writeImageDataToPasteboard(NSDictionary <NSString *, NSData *> *typesAndData)
     75{
     76    UIPasteboard.generalPasteboard.items = @[ typesAndData ];
     77}
     78
     79void writeImageDataToPasteboard(NSArray<NSDictionary <NSString *, NSData *> *> *items)
     80{
     81    UIPasteboard.generalPasteboard.items = items;
     82}
     83
    5084#endif
    5185
    … …  
    138172}
    139173
     174TEST(PasteImage, PasteImageWithMultipleRepresentations)
     175{
     176    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
     177    [webView synchronouslyLoadTestPageNamed:@"paste-image"];
     178
     179    auto pngData = [NSData dataWithContentsOfFile:[[NSBundle mainBundle] pathForResource:@"sunset-in-cupertino-200px" ofType:@"png" inDirectory:@"TestWebKitAPI.resources"]];
     180    auto jpegData = [NSData dataWithContentsOfFile:[[NSBundle mainBundle] pathForResource:@"sunset-in-cupertino-600px" ofType:@"jpg" inDirectory:@"TestWebKitAPI.resources"]];
     181    writeImageDataToPasteboard(@{
     182        (__bridge NSString *)kUTTypePNG : pngData,
     183        (__bridge NSString *)kUTTypeJPEG : jpegData
     184    });
     185    [webView paste:nil];
     186
     187    EXPECT_WK_STREQ("1", [webView stringByEvaluatingJavaScript:@"dataTransfer.files.length"]);
     188
     189    writeImageDataToPasteboard(@[
     190        @{ (__bridge NSString *)kUTTypePNG : pngData },
     191        @{ (__bridge NSString *)kUTTypeJPEG : jpegData }
     192    ]);
     193    [webView paste:nil];
     194
     195    EXPECT_WK_STREQ("2", [webView stringByEvaluatingJavaScript:@"dataTransfer.files.length"]);
     196}
     197
    140198TEST(PasteImage, RevealSelectionAfterPastingImage)
    141199{
Note: See TracChangeset for help on using the changeset viewer.