Bug 167578

Summary: [css-grid] Enable CSS Grid Layout by default
Product: WebKit Reporter: Manuel Rego Casasnovas <rego>
Component: WebKit2Assignee: Manuel Rego Casasnovas <rego>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, commit-queue, darin, jfernandez, mcatanzaro, svillar
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 60731    
Attachments:
Description Flags
Patch none

Description Manuel Rego Casasnovas 2017-01-30 02:06:37 PST
Now that Safari 10.1 is shipping CSS Grid Layout it seems safe to enable the feature by default:
https://developer.apple.com/library/prerelease/content/releasenotes/General/WhatsNewInSafari/Articles/Safari_10_1.html
Comment 1 Manuel Rego Casasnovas 2017-01-30 02:07:48 PST
Created attachment 300092 [details]
Patch
Comment 2 Michael Catanzaro 2017-01-30 06:33:52 PST
Comment on attachment 300092 [details]
Patch

\o/

But you forgot a part: Source/WebKit2/UIProcess/gtk/ExperimentalFeatures.[h,cpp]. You have to remove CSS grid layout from the settings array as well, since it's no longer experimental. But then we would have no more experimental features covered by this file, so it makes sense to remove the entire file. You could do this in a separate patch, but we should land them at the same time. I wanted to remove it anyway because it needs to be rewritten to use the enumerable experimental features API, so it covers all experimental features instead of just being one more place they have to be hardcoded, and I don't think it makes sense to keep it around in the meantime.
Comment 3 Manuel Rego Casasnovas 2017-01-30 07:33:14 PST
(In reply to comment #2)
> But you forgot a part:
> Source/WebKit2/UIProcess/gtk/ExperimentalFeatures.[h,cpp]. You have to
> remove CSS grid layout from the settings array as well, since it's no longer
> experimental. But then we would have no more experimental features covered
> by this file, so it makes sense to remove the entire file. You could do this
> in a separate patch, but we should land them at the same time. I wanted to
> remove it anyway because it needs to be rewritten to use the enumerable
> experimental features API, so it covers all experimental features instead of
> just being one more place they have to be hardcoded, and I don't think it
> makes sense to keep it around in the meantime.

True, let's do it in a different patch: bug #167586
Comment 4 Manuel Rego Casasnovas 2017-01-30 12:24:24 PST
Thanks for the review.

I'll give some time to other people to comment on this.
Otherwise I'll land it tomorrow.
Comment 5 WebKit Commit Bot 2017-01-31 01:25:17 PST
Comment on attachment 300092 [details]
Patch

Clearing flags on attachment: 300092

Committed r211415: <http://trac.webkit.org/changeset/211415>
Comment 6 WebKit Commit Bot 2017-01-31 01:25:20 PST
All reviewed patches have been landed.  Closing bug.