Bug 157391 - [GTK] Add CSS Grid Layout as experimental feature
Summary: [GTK] Add CSS Grid Layout as experimental feature
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Manuel Rego Casasnovas
URL:
Keywords:
Depends on:
Blocks: 157393
  Show dependency tree / graph
 
Reported: 2016-05-05 15:08 PDT by Manuel Rego Casasnovas
Modified: 2016-05-24 05:55 PDT (History)
8 users (show)

See Also:

Attachments
Patch (3.10 KB, patch)
2016-05-05 15:09 PDT, Manuel Rego Casasnovas 		no flags Details | Formatted Diff | Diff
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 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.