[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 #4 from Ryosuke Niwa <rniwa at webkit.org>  2011-04-23 16:50:53 PST ---
(From update of attachment 90859)
View in context: https://bugs.webkit.org/attachment.cgi?id=90859&action=review

Exciting patch! I know you're not posting for a review but I concur with Morrita-san's points. The current definition of findDropZone is really long and should be split into smaller functions.

> Source/WebCore/ChangeLog:11
> +        element and continue processing the drag and drop operation.

It'll be nice to have a link to the relevant spec.

> Source/WebCore/page/EventHandler.cpp:1779
> +    Element* element = target->isElementNode() ? toElement(target) : target->parentElement();
> +    bool matched = false;
> +    for (; element && !matched; element = element->parentElement()) {

What if the hit test ended inside a shadow DOM?

> Source/WebCore/page/EventHandler.cpp:1809
> +            if (keywords[i].length() < 3)
> +                continue;
> +            if (keywords[i][1] != ':')
> +                continue;
> +            if (keywords[i][0] == 's')
> +                isFile = false;
> +            else if (keywords[i][0] == 'f')
> +                isFile = true;
> +            else
> +                continue;
> +            type = keywords[i].string().substring(2).lower();

I have no idea what this code is trying to do.  As Morrita-san said, you should extract this code into a helper function that convers string into an enum or bit flags.

> Source/WebCore/page/EventHandler.cpp:1812
> +            if (!isFile) {

It seems like things in if/else clauses can each be a separate function.

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