Bug 33738 – DataTransfer interface does not work on Windows

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Platform

[Daniel Cheng]

The format enumerator isn't being called correctly in ClipboardWin::types and ClipboardWin::hasData.

[Daniel Cheng]

Created attachment 46711 [details]
Patch for the problems described

[Adam Roben (:aroben)]

Comment on attachment 46711 [details]
Patch for the problems described

> +        Not sure how to add a good test for it.

I'll bet we can come up with a test. If .types and .getData are completely broken, it should be pretty easy! There are probably existing tests you can build off of; grep through LayoutTests for uses of event.dataTransfer.

> +        * platform/win/ClipboardWin.cpp:
> +        (WebCore::addMimeTypesForFormat):
> +        (WebCore::ClipboardWin::types):
> +        (WebCore::ClipboardWin::hasData):

You should add function-level comments about what your patch does and why.

> @@ -568,7 +568,7 @@ HashSet<String> ClipboardWin::types() co
>  
>      FORMATETC data;
>  
> -    while (SUCCEEDED(itr->Next(1, &data, 0))) {
> +    while (itr->Next(1, &data, 0) == S_OK) {
>          addMimeTypesForFormat(results, data);
>      }
>  
> @@ -789,7 +789,7 @@ bool ClipboardWin::hasData()
>  
>      FORMATETC data;
>  
> -    if (SUCCEEDED(itr->Next(1, &data, 0))) {
> +    if (itr->Next(1, &data, 0) == S_OK) {
>          // There is at least one item in the IDataObject
>          return true;
>      }

It might be worth adding a comment about the return value of IEnumFORMATETC::Next.

The code changes look good, but r- so we can come up with a test and fix up the ChangeLog.

[Daniel Cheng]

Created attachment 46719 [details]
Patch

Updated ChangeLog, added a few comments in the code, and updated a layout test.

As it turns out, fast/events/drag-and-drop-dataTransfer-types-nocrash.html already partially covers this so I extended it to test event.dataTransfer.getData() as well.

[Adam Roben (:aroben)]

Comment on attachment 46719 [details]
Patch

This looks great! The one thing we still need to do is actually enable that test on Windows. You will find it listed in LayoutTests/platform/win/Skipped. Please remove it from that file and resubmit your patch and we can get this committed!

[Adam Roben (:aroben)]

Duping to bug 30527. Daniel, please attach your next revision of the patch to that bug.

*** This bug has been marked as a duplicate of bug 30527 ***