Bug 163432

Summary: [css-grid] Disable CSS Grid Layout runtime flag by default
Product: WebKit Reporter: Manuel Rego Casasnovas <rego>
Component: CSSAssignee: Manuel Rego Casasnovas <rego>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, commit-queue, dino, jfernandez, joepeck, rego, 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 2016-10-14 00:52:00 PDT
In r201042 the runtime flags were enabled by default as Safari Technology Preview was lacking a UI to modify them.

Now that it has a UI to enable/disable the flags we don't need that Grid Layout is enabled by default anymore.
Comment 1 Manuel Rego Casasnovas 2016-10-14 00:55:00 PDT
Created attachment 291588 [details]
Patch
Comment 2 WebKit Commit Bot 2016-10-17 00:25:48 PDT
Comment on attachment 291588 [details]
Patch

Clearing flags on attachment: 291588

Committed r207402: <http://trac.webkit.org/changeset/207402>
Comment 3 WebKit Commit Bot 2016-10-17 00:25:52 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Joseph Pecoraro 2016-10-17 16:37:19 PDT
This change caused some buttons in Web Inspector to break their position!! That said, I don't think we should roll this out, I think this is just exposing an issue that this runtime flag is affecting some things that it should be affecting. I'll investigate.
Comment 5 Sergio Villar Senin 2016-10-18 05:40:49 PDT
(In reply to comment #4)
> This change caused some buttons in Web Inspector to break their position!!
> That said, I don't think we should roll this out, I think this is just
> exposing an issue that this runtime flag is affecting some things that it
> should be affecting. I'll investigate.

Is web inspector using grid layout? Sounds weird specially because we have not shipped it yet.
Comment 6 Javier Fernandez 2016-10-18 06:05:55 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > This change caused some buttons in Web Inspector to break their position!!
> > That said, I don't think we should roll this out, I think this is just
> > exposing an issue that this runtime flag is affecting some things that it
> > should be affecting. I'll investigate.
> 
> Is web inspector using grid layout? Sounds weird specially because we have
> not shipped it yet.

The bug #163572 has been filed precisely to track this issue. 

The problem is not grid itself, but the CSS Box Alignment properties it shares
with Flexbox. Due to the different behavior of the new syntax, we need to use the runtime and compile flags to implement both code paths.

BTW, there is already a proposed fix for bug #163572, so we can continue discussing the details there.