| Summary: | Add missing ENABLE(DRAG_SUPPORT) guards for DragClient and clients | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Brian Burg <burg> | ||||||||
| Component: | Images | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED WONTFIX | ||||||||||
| Severity: | Normal | CC: | bburg, darin, eflews.bot, gyuyoung.kim | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 125248 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Brian Burg
2013-12-06 14:25:25 PST
Created attachment 218707 [details]
patch
Comment on attachment 218707 [details] patch Attachment 218707 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/46838079 Created attachment 218712 [details]
patch
Comment on attachment 218712 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=218712&action=review The cross-platform changes here are good; it’s a small refinement that costs very little to cut out even more of the drag code for platforms that have it turned off. But the platform-specific part of this patch is overkill. It’s a mistake to add ENABLE(DRAG_SUPPORT) conditionals for every platform. Most platforms compile only with drag support enabled or only with drag support disabled. We don’t need each platform to support both conditions. Unfortunately I don’t know the status of the not-Apple-driven ports such as GTK, EFL, and WinCE. > Source/WebCore/page/Page.cpp:1563 > , dragClient(0) Should be nullptr instead of 0. > Source/WebKit/win/WebCoreSupport/WebDragClient.h:31 > +#if ENABLE(DRAG_SUPPORT) No need to do this. Windows should always be compiled with DRAG_SUPPORT enabled. > Source/WebKit/win/WebView.cpp:2770 > +#if ENABLE(DRAG_SUPPORT) No need to do this. Windows should always be compiled with DRAG_SUPPORT enabled. > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:191 > +#if ENABLE(DRAG_SUPPORT) > +#include "WebDragClient.h" > +#include <WebCore/DragController.h> > +#include <WebCore/DragData.h> > +#include <WebCore/DragSession.h> > +#endif This change isn’t needed. These headers themselves are already properly conditionalized and there is no harm in including them unconditionally. (In reply to comment #4) > (From update of attachment 218712 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=218712&action=review > > The cross-platform changes here are good; it’s a small refinement that costs very little to cut out even more of the drag code for platforms that have it turned off. > > But the platform-specific part of this patch is overkill. OK, noted. I'll fix the other patches to heed this too. Created attachment 218931 [details]
patch
Not needed. |