[webkit-reviews] review denied: [Bug 105041] Allow disabling drag-and-drop at runtime : [Attachment 179500] Patch

[bugzilla-daemon at webkit.org]

Maciej Stachowiak <mjs at apple.com> has denied Sadrul Habib Chowdhury
<sadrul at chromium.org>'s request for review:
Bug 105041: Allow disabling drag-and-drop at runtime
https://bugs.webkit.org/show_bug.cgi?id=105041

Attachment 179500: Patch
https://bugs.webkit.org/attachment.cgi?id=179500&action=review

------- Additional Comments from Maciej Stachowiak <mjs at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=179500&action=review


r- to address comments above. It might be that this functionality is useful but
the motivation is not explained, and the patch doesn't seem to do things in a
clean way.

> Source/WebCore/ChangeLog:9
> +	   It may be desirable to disallow drag-n-drop for particular pages at
runtime. This
> +	   change introduces Page::resetDragController() for such cases.

Why would it be desirable?

And isn't it already achievable using regular web platform features, such as
setting capturing event listeners on the window for all drag/drop events which
prevents default and stops propagation?

> Source/WebCore/page/Page.cpp:245
> +    m_dragController.clear();

Nulling out the drag controller seems like a poor way to represent a "don't
allow any drags to start in this page" flag. It's in-band signaling. And it
means that any code path that forgets to do the proper check will crash instead
of inadvertently doing dragging. An explicit flag would only need to be checked
in a few places (ones that can start a drag) instead of in all code paths that
can interact with DnD.