Bug 157391

Summary: [GTK] Add CSS Grid Layout as experimental feature
Product: WebKit Reporter: Manuel Rego Casasnovas <rego>
Component: WebKitGTKAssignee: Manuel Rego Casasnovas <rego>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bugs-noreply, cgarcia, commit-queue, gustavo, mcatanzaro, mrobinson, svillar
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 157393    
Attachments:
Description Flags
Patch none

Description Manuel Rego Casasnovas 2016-05-05 15:08:22 PDT
Now that we've a runtime feature for CSS Grid Layout again (bug #157134),
we can recover the old patch to enable it from an env variable (bug #127089).
Comment 1 Manuel Rego Casasnovas 2016-05-05 15:09:30 PDT
Created attachment 278194 [details]
Patch
Comment 2 WebKit Commit Bot 2016-05-05 15:14:17 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Michael Catanzaro 2016-05-05 16:09:05 PDT
Comment on attachment 278194 [details]
Patch

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

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:157
> +    ExperimentalFeatures features;

I usually prefer to leave one blank line above variable declarations, especially when they're not initialized. It's easier to read IMO.
Comment 4 WebKit Commit Bot 2016-05-05 16:59:13 PDT
Comment on attachment 278194 [details]
Patch

Clearing flags on attachment: 278194

Committed r200497: <http://trac.webkit.org/changeset/200497>
Comment 5 WebKit Commit Bot 2016-05-05 16:59:17 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Michael Catanzaro 2016-05-15 17:56:24 PDT
The compile flag is still off by default. I don't think the runtime flag serves any useful purpose if this functionality is compiled out by default. Presumably, you would only change the build flags if you really want to test CSS grid layout, so it's currently just another obstacle to overcome.

I think we should flip ENABLE_CSS_GRID_LAYOUT to be on by default.
Comment 7 Manuel Rego Casasnovas 2016-05-24 02:09:31 PDT
(In reply to comment #6)
> The compile flag is still off by default. I don't think the runtime flag
> serves any useful purpose if this functionality is compiled out by default.
> Presumably, you would only change the build flags if you really want to test
> CSS grid layout, so it's currently just another obstacle to overcome.
> 
> I think we should flip ENABLE_CSS_GRID_LAYOUT to be on by default.

Yeah I agree, at least for WebKitGTK+ we want to build it by default (and keep the runtime flag disabled by default).

Should we modify Source/cmake/WebKitFeatures.cmake to make it ON by default?
Or the change should go somewhere else?
Comment 8 Michael Catanzaro 2016-05-24 05:55:25 PDT
(In reply to comment #7)
> Should we modify Source/cmake/WebKitFeatures.cmake to make it ON by default?

Yes.

Also in FeatureList.pm if it's not already.