Bug 73594 – [chromium] Add plumbing for supporting custom MIME types in DataTransfer.

• Component: New Bugs
• Status: RESOLVED FIXED

[Daniel Cheng]

[chromium] Add plumbing for supporting custom MIME types in DataTransfer.

[Daniel Cheng]

Created attachment 117500 [details]
Patch

[WebKit Review Bot]

Attachment 117500 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/edit..." exit_code: 1

ERROR: FAILURES FOR <lucid, x86_64, release, cpu> in LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:1953 Duplicate or ambiguous expectation. fast/url/file-http-base.html
LayoutTests/platform/chromium/test_expectations.txt:1953:  Duplicate or ambiguous expectation. fast/url/file-http-base.html  [test/expectations] [5]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Tony Chang]

Comment on attachment 117500 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117500&action=review

> Source/WebCore/platform/chromium/ChromiumDataObject.cpp:187
> +
>      return String();

I think we still need to set success = false; here.

> Source/WebCore/platform/chromium/ChromiumDataObject.cpp:237
> +    m_customData.set(type, data);
> +    return true;

I see, so setData will always succeed now.

> LayoutTests/platform/chromium/test_expectations.txt:3738
> +// Depends on Chromium follow up change before they will pass.
> +BUG_DCHENG : fast/events/drag-customData.html = FAIL

Maybe just reference BUGCR31037 here?

[Tony Chang]

Comment on attachment 117500 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117500&action=review

>> Source/WebCore/platform/chromium/ChromiumDataObject.cpp:237
>> +    return true;
> 
> I see, so setData will always succeed now.

Actually, should you verify that type is not an empty string?

[Daniel Cheng]

Created attachment 117507 [details]
Patch

[WebKit Review Bot]

Attachment 117507 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/edit..." exit_code: 1

ERROR: FAILURES FOR <lucid, x86_64, release, cpu> in LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:1953 Duplicate or ambiguous expectation. fast/url/file-http-base.html
LayoutTests/platform/chromium/test_expectations.txt:1953:  Duplicate or ambiguous expectation. fast/url/file-http-base.html  [test/expectations] [5]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Daniel Cheng]

Created attachment 117509 [details]
Patch

[Daniel Cheng]

(In reply to comment #5)
> (From update of attachment 117500 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117500&action=review
> 
> >> Source/WebCore/platform/chromium/ChromiumDataObject.cpp:237
> >> +    return true;
> > 
> > I see, so setData will always succeed now.
> 
> Actually, should you verify that type is not an empty string?

Done.

(In reply to comment #4)
> (From update of attachment 117500 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117500&action=review
> 
> > Source/WebCore/platform/chromium/ChromiumDataObject.cpp:187
> > +
> >      return String();
> 
> I think we still need to set success = false; here.
> 

Done.

> > Source/WebCore/platform/chromium/ChromiumDataObject.cpp:237
> > +    m_customData.set(type, data);
> > +    return true;
> 
> I see, so setData will always succeed now.
> 
> > LayoutTests/platform/chromium/test_expectations.txt:3738
> > +// Depends on Chromium follow up change before they will pass.
> > +BUG_DCHENG : fast/events/drag-customData.html = FAIL
> 
> Maybe just reference BUGCR31037 here?

Done.

[WebKit Review Bot]

Attachment 117509 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/edit..." exit_code: 1

ERROR: FAILURES FOR <lucid, x86_64, release, cpu> in LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:1953 Duplicate or ambiguous expectation. fast/url/file-http-base.html
LayoutTests/platform/chromium/test_expectations.txt:1953:  Duplicate or ambiguous expectation. fast/url/file-http-base.html  [test/expectations] [5]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tony Chang]

Comment on attachment 117509 [details]
Patch

LGTM, but please give fishd a chance to approve the API addition.

[WebKit Review Bot]

Comment on attachment 117509 [details]
Patch

Attachment 117509 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10693830

[Darin Fisher (:fishd, Google)]

Comment on attachment 117509 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117509&action=review

> Source/WebKit/chromium/public/platform/WebClipboard.h:82
> +        Buffer buffer, const WebString& type) { return WebString(); }

nit: Buffer parameter should be unnamed

> Source/WebKit/chromium/public/platform/WebDragData.h:102
> +        WebString m_type;

nit: public members should not have the m_ prefix.

[Daniel Cheng]

Created attachment 117572 [details]
Patch

[Daniel Cheng]

(In reply to comment #13)
> (From update of attachment 117509 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117509&action=review
> 
> > Source/WebKit/chromium/public/platform/WebClipboard.h:82
> > +        Buffer buffer, const WebString& type) { return WebString(); }
> 
> nit: Buffer parameter should be unnamed
> 

Done.


> > Source/WebKit/chromium/public/platform/WebDragData.h:102
> > +        WebString m_type;
> 
> nit: public members should not have the m_ prefix.

Done.

[WebKit Review Bot]

Comment on attachment 117572 [details]
Patch

Attachment 117572 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10725258

New failing tests:
editing/pasteboard/clipboard-customData.html

[Daniel Cheng]

Created attachment 117580 [details]
Patch

[David Levin]

Comment on attachment 117580 [details]
Patch

I'll do the final honors. It looks like you have a looks good on the code and the public api.

[Daniel Cheng]

Committed r101828: <http://trac.webkit.org/changeset/101828>