[Webkit-unassigned] [Bug 33738] DataTransfer interface does not work on Windows

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=33738


Adam Roben (aroben) <aroben at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #46711|review?                     |review-
               Flag|                            |




--- Comment #2 from Adam Roben (aroben) <aroben at apple.com>  2010-01-15 14:43:42 PST ---
(From update of attachment 46711)
> +        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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.