[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 #3 from MORITA Hajime <morrita at google.com>  2011-04-23 16:07:32 PST ---
(From update of attachment 90859)
View in context: https://bugs.webkit.org/attachment.cgi?id=90859&action=review

Hi, I tried to make some comment. Many of these will be pointless for your review-ready version though ;-)

In my preference, every code other than element traversal and dropZone value retrieval doen't need to be a part of EventHandler.
EventHandler is already complex enough and it will be good to keep this class small.

> Source/WebCore/page/EventHandler.cpp:1794
> +            if (equalIgnoringCase(keywords[i], "copy") || equalIgnoringCase(keywords[i], "move") || equalIgnoringCase(keywords[i], "link")) {

How about to define a string to enum conversion function?
This type checking facility looks a bit hacky.

> Source/WebCore/page/EventHandler.cpp:1815
> +                if (dragState().m_dragSrcIsDHTML)

How about to ask DragState to choose appropriate clipboard object?
The dragState object looks to know everything to decide it.

> Source/WebCore/page/EventHandler.cpp:1838
> +                for (unsigned int f = 0; f < clipboard->files()->length() && !hasType; f++) {

How about to ask hasType() to Clipboard?

> Source/WebCore/page/EventHandler.cpp:1849
> +            matched = true;

How about just break; ?

> Source/WebCore/page/EventHandler.cpp:1889
> +            if (!accept)

Why isn't findDropZone() a part of (or called inside) dispatchDragEvent() ?

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