Bug 125369 - Fix build for PLATFORM(MAC) && !ENABLE(DRAG_SUPPORT)
Summary: Fix build for PLATFORM(MAC) && !ENABLE(DRAG_SUPPORT)
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 125248
  Show dependency tree / graph
 
Reported: 2013-12-06 14:56 PST by Brian Burg
Modified: 2013-12-08 11:56 PST (History)
5 users (show)

See Also:

Attachments
patch (7.54 KB, patch)
2013-12-06 17:30 PST, Brian Burg 		no flags Details | Formatted Diff | Diff
v1 (5.55 KB, patch)
2013-12-06 18:59 PST, Brian Burg 		darin: review-
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Burg 2013-12-06 14:56:30 PST 
Some missing guards have accreted over time.
Comment 1 Brian Burg 2013-12-06 17:30:29 PST 
Created attachment 218634 [details]
patch
Comment 2 Build Bot 2013-12-06 18:31:59 PST 
Comment on attachment 218634 [details]
patch

Attachment 218634 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/46318001
Comment 3 Brian Burg 2013-12-06 18:58:15 PST 
Comment on attachment 218634 [details]
patch

bad patch
Comment 4 Brian Burg 2013-12-06 18:59:48 PST 
Created attachment 218643 [details]
v1
Comment 5 Darin Adler 2013-12-06 20:01:59 PST 
Comment on attachment 218643 [details]
v1

View in context: https://bugs.webkit.org/attachment.cgi?id=218643&action=review

> Source/WebCore/ChangeLog:3
> +        Build fix for PLATFORM(MAC) && !ENABLE(DRAG_SUPPORT)

I don’t think this is helpful to do. We don’t need to support all combinations of ENABLE and PLATFORM. We do want to build for iOS without drag supported, but I see no reason we have to support non-iOS PLATFORM(MAC) with drag support off.

> Source/WebCore/dom/ClipboardMac.mm:29
> +#if ENABLE(DRAG_SUPPORT)

Not helpful to add this.

> Source/WebCore/platform/DragImage.cpp:-29
> -#if ENABLE(DRAG_SUPPORT)

Fine to remove this.

> Source/WebCore/platform/mac/DragImageMac.mm:-29
> -#if ENABLE(DRAG_SUPPORT)

Fine to remove this.

> Source/WebCore/platform/mac/PasteboardMac.mm:144
> +#if ENABLE(DRAG_SUPPORT)

Not helpful to add this.

> Source/WebCore/platform/mac/PasteboardMac.mm:645
> +#if ENABLE(DRAG_SUPPORT)

Not helpful to add this.
Comment 6 Brian Burg 2013-12-07 00:17:10 PST 
Thanks for the quick turnaround.

(In reply to comment #5)
> (From update of attachment 218643 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=218643&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        Build fix for PLATFORM(MAC) && !ENABLE(DRAG_SUPPORT)
> 
> I don’t think this is helpful to do. We don’t need to support all combinations of ENABLE and PLATFORM. We do want to build for iOS without drag supported, but I see no reason we have to support non-iOS PLATFORM(MAC) with drag support off.

I really just want the ENABLE(DRAG_SUPPORT) guards to be accurate in DragImage.h. However, changing DragImage.{cpp,h} first will cascade into also fixing DragData, DragClient, and other stuff, so I wanted to split up the patches. This first patch was just getting me to a baseline where I can test !ENABLE(DRAG_SUPPORT) from a non-windows environment. The other patches fix guards for DragData, DragClient, and DragImage mostly independently.
Comment 7 Brian Burg 2013-12-08 11:56:52 PST 
I'll just close this bug since the changes are really only helpful for my testing of the other bugs, not for anyone else.