Bug 114396 - [Windows, WinCairo] FeatureDefines.h Overriding Settings in VS Property Sheets
Summary: [Windows, WinCairo] FeatureDefines.h Overriding Settings in VS Property Sheets
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-10 17:09 PDT by Brent Fulgham
Modified: 2013-06-13 10:48 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.11 KB, patch)
2013-06-13 10:40 PDT, Brent Fulgham
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2013-04-10 17:09:43 PDT
The FeatureDefines.h file forces certain features to be active, regardless of the FeatureDefines.prop contents.  This may be a historical issue, from a time when the property sheet did not exist and was not used to control feature activations.

I think it is confusing (and wrong) for this header file to force any of the features to be turned on, if those features are defined and handled in the property sheet.
Comment 1 Brent Fulgham 2013-04-10 17:12:07 PDT
On the Apple Windows target, ENABLE_VIEW_MODE_CSS_MEDIA is included in compiler's command line output, e.g, /D "ENABLE_VIEW_MODE_CSS_MEDIA" because it is marked as "active" in the FeatureDefines.props file.

On the WInCairo build, we have ENABLE_VIEW_MODE_CSS_MEDIA turned off at the moment (in the FeatureDefinesCairo.props file).  Unfortunately, FeatureDefines.h silently flips it back on.

I think the various defines for OS(WINDOWS) in the FeatureDefines.h file that automatically activate certain features should be removed, unless the GTK-Windows, WinCE, wxWidgets, or some other port needs them.
Comment 2 Patrick R. Gansterer 2013-04-10 21:00:20 PDT
(In reply to comment #1)
> I think the various defines for OS(WINDOWS) in the FeatureDefines.h file that automatically activate certain features should be removed, unless the GTK-Windows, WinCE, wxWidgets, or some other port needs them.

No needs in the WinCE port.
Comment 3 Brent Fulgham 2013-06-13 10:40:48 PDT
Created attachment 204627 [details]
Patch
Comment 4 Brent Fulgham 2013-06-13 10:41:32 PDT
I misunderstood how this file is being used.  I think the right solution here is to add a default case for WinCairo that sets the ENABLE_VIEW_MODE_CSS_MEDIA flag to 0 for the WinCairo port.
Comment 5 Brent Fulgham 2013-06-13 10:48:30 PDT
Committed r151561: <http://trac.webkit.org/changeset/151561>