[Webkit-unassigned] [Bug 58210] webkit should implement the dropzone attribute

[bugzilla-daemon at webkit.org]

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





--- Comment #17 from Daniel Cheng <dcheng at chromium.org>  2011-04-29 01:25:05 PST ---
(From update of attachment 91608)
View in context: https://bugs.webkit.org/attachment.cgi?id=91608&action=review

I am not a WebKit reviewer, so these are just my thoughts.

> Source/WebCore/dom/Clipboard.cpp:162
> +bool Clipboard::hasFileOfType(const String& type) const

For consistency, should these new methods check that the policy of the clipboard matches what is expected?

> Source/WebCore/dom/Clipboard.cpp:168
> +    for (unsigned int f = 0; f < files()->length(); f++) {

You can use fileList here instead of calling files().

> Source/WebCore/dom/Clipboard.cpp:205
> +bool Clipboard::processDropZoneKeyword(const String& keyword)

Nit: I would keep the order of definitions the same as the order of declarations in the header file.

> Source/WebCore/dom/Clipboard.h:97
> +        bool hasStringOfType(const String&) const;

Should these methods be private since only Clipboard calls them?

> Source/WebCore/dom/Clipboard.h:98
> +        bool processDropZoneKeyword(const String&);

This method might be more clearly named hasDropZoneType.

> Source/WebCore/page/EventHandler.cpp:1796
> +                    clipboard->setDropEffect(convertDragOperationToDropZoneOperation(dragOperation));

It seems redundant to have call Clipboard::setDropEffect in 3 places. Can you consolidate it into one return point for the successfully matched case?

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