[Webkit-unassigned] [Bug 58210] webkit should implement the dropzone attribute
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Apr 23 18:47:55 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=58210
--- Comment #5 from Yael <yael.aharon at nokia.com> 2011-04-23 18:47:55 PST ---
(In reply to comment #3)
> (From update of attachment 90859 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=90859&action=review
>
Thank you for reviewing
> 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.
>
Agree. I was waiting for rniwa's hacking session on EventHandler to see what changes need to happen.
> > 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.
>
I started out with using the enum DropOperation, but then I decided to change because this is the only place it is used. I will change back.
> > 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.
>
ok
> > Source/WebCore/page/EventHandler.cpp:1838
> > + for (unsigned int f = 0; f < clipboard->files()->length() && !hasType; f++) {
>
> How about to ask hasType() to Clipboard?
>
ok
> > Source/WebCore/page/EventHandler.cpp:1849
> > + matched = true;
>
> How about just break; ?
>
I can't just break because the drop operation could come after the matching type was found. I could probably break if aI added more checks throughout the function.
> > Source/WebCore/page/EventHandler.cpp:1889
> > + if (!accept)
>
> Why isn't findDropZone() a part of (or called inside) dispatchDragEvent() ?
We don't call findDropZone() if the event is DragLeave, Drag or Drop.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list