Bug 130013 – [CSS Grid Layout] Enable runtime feature if feature flag is enabled

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Manuel Rego Casasnovas]

Currently in development builds ENABLE_CSS_GRID_LAYOUT is true, however the runtime feature is still false so you cannot test it without doing a manual build.

It would be nice to enable the runtime flag when ENABLE_CSS_GRID_LAYOUT is true, otherwise it doesn't have too much sense to compile the core related to CSS Grid Layout if  you're not going to be able to use it.

[Manuel Rego Casasnovas]

Created attachment 226292 [details]
Patch

[Build Bot]

Comment on attachment 226292 [details]
Patch

Attachment 226292 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5520739989979136

New failing tests:
fast/css-grid-layout/grid-disabled-by-default.html

[Build Bot]

Created attachment 226294 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Manuel Rego Casasnovas]

Created attachment 226299 [details]
Patch

Removing fast/css-grid-layout/grid-disabled-by-default.html as it fails with this patch.

[Manuel Rego Casasnovas]

Created attachment 226413 [details]
Patch

Finally keeping layout test but checking that the runtime feature can be disabled.

[Simon Fraser (smfr)]

Comment on attachment 226413 [details]
Patch

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

> Source/WebKit2/Shared/WebPreferencesStore.h:105
> +#if ENABLE(CSS_GRID_LAYOUT)
> +#define DEFAULT_CSS_GRID_LAYOUT_ENABLED true
> +#else
> +#define DEFAULT_CSS_GRID_LAYOUT_ENABLED false
> +#endif

This is only changing WebKit2. What about WebKit1?

[Manuel Rego Casasnovas]

Created attachment 226957 [details]
Patch

Changing WK1 code too.

[Manuel Rego Casasnovas]

(In reply to comment #7)
> Created an attachment (id=226957) [details]
> Patch
> 
> Changing WK1 code too.

Anyway, maybe it would be a better idea to make the runtime feature true directly in WebCore/page/Settings.in.

So if you compile with ENBALE_CSS_GRID_LAYOUT the feature will be enabled by default (and you have still the chance to disable the runtime feature if needed). And if you don't compile with ENBALE_CSS_GRID_LAYOUT the feature won't work as expected.

If you prefer this kind of change, I can change the patch and do something like that. What do you think?

[WebKit Commit Bot]

Comment on attachment 226957 [details]
Patch

Clearing flags on attachment: 226957

Committed r166614: <http://trac.webkit.org/changeset/166614>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.