Bug 167693 - [css-grid] Remove compilation flag ENABLE_CSS_GRID_LAYOUT
Summary: [css-grid] Remove compilation flag ENABLE_CSS_GRID_LAYOUT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Manuel Rego Casasnovas
URL:
Keywords:
Depends on:
Blocks: 60731
  Show dependency treegraph
 
Reported: 2017-02-01 09:25 PST by Manuel Rego Casasnovas
Modified: 2017-02-20 02:03 PST (History)
7 users (show)

See Also:


Attachments
Patch (151.23 KB, patch)
2017-02-01 09:30 PST, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Apply suggested changes (151.54 KB, patch)
2017-02-08 05:27 PST, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (725.31 KB, application/zip)
2017-02-08 07:13 PST, Build Bot
no flags Details
Rebased patch for landing (150.70 KB, patch)
2017-02-20 01:24 PST, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Manuel Rego Casasnovas 2017-02-01 09:25:30 PST
Now that CSS Grid Layout is enabled by default (r211415) and Safari 10.1 is shipping it [1],
it seems we might not need the compilation flag anymore.

[1] https://developer.apple.com/library/prerelease/content/releasenotes/General/WhatsNewInSafari/Articles/Safari_10_1.html
Comment 1 Manuel Rego Casasnovas 2017-02-01 09:30:04 PST
Created attachment 300330 [details]
Patch
Comment 2 Sergio Villar Senin 2017-02-06 01:45:24 PST
Comment on attachment 300330 [details]
Patch

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

Nice work. I guess that having a runtime preference won't make much sense either, but we could remove it in a follow up patch.

I'm giving r+ but wait some time for an Apple review (although I guess they'll be fine with the change as Safari is shipping it).

> Source/WebCore/dom/Position.cpp:1016
>          || is<RenderFlexibleBox>(*renderer)) {

Perhaps you could put these three in a single line now
Comment 3 Manuel Rego Casasnovas 2017-02-08 05:27:20 PST
Created attachment 300891 [details]
Apply suggested changes
Comment 4 Manuel Rego Casasnovas 2017-02-08 05:29:42 PST
Comment on attachment 300330 [details]
Patch

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

Thanks for the review.

Now let's wait for some feedback from Apple reviewers.

>> Source/WebCore/dom/Position.cpp:1016
>>          || is<RenderFlexibleBox>(*renderer)) {
> 
> Perhaps you could put these three in a single line now

Done.
Comment 5 Build Bot 2017-02-08 07:13:30 PST
Comment on attachment 300891 [details]
Apply suggested changes

Attachment 300891 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3025048

New failing tests:
compositing/masks/solid-color-masked.html
Comment 6 Build Bot 2017-02-08 07:13:33 PST
Created attachment 300894 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 7 Manuel Rego Casasnovas 2017-02-13 01:12:50 PST
Patch is ready for landing and waiting for Apple's feedback.

If nobody complains during this week, I think it's safe we land it.
Comment 8 Manuel Rego Casasnovas 2017-02-20 01:24:27 PST
Created attachment 302124 [details]
Rebased patch for landing
Comment 9 WebKit Commit Bot 2017-02-20 02:03:08 PST
Comment on attachment 302124 [details]
Rebased patch for landing

Clearing flags on attachment: 302124

Committed r212629: <http://trac.webkit.org/changeset/212629>
Comment 10 WebKit Commit Bot 2017-02-20 02:03:13 PST
All reviewed patches have been landed.  Closing bug.