Bug 170764 - [css-grid] Add support for percentage gaps
Summary: [css-grid] Add support for percentage gaps
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Manuel Rego Casasnovas
URL:
Keywords: BlinkMergeCandidate
Depends on: 168657
Blocks: 60731
  Show dependency treegraph
 
Reported: 2017-04-11 22:45 PDT by Manuel Rego Casasnovas
Modified: 2017-04-18 09:27 PDT (History)
8 users (show)

See Also:


Attachments
Patch (37.48 KB, patch)
2017-04-11 22:58 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Mini change on ChangeLog (37.49 KB, patch)
2017-04-12 01:40 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (885.64 KB, application/zip)
2017-04-12 02:45 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (865.02 KB, application/zip)
2017-04-12 04:00 PDT, Build Bot
no flags Details
Patch for landing (37.15 KB, patch)
2017-04-18 08:42 PDT, 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-04-11 22:45:10 PDT
This has been already implemented in Blink.
Part of the code related to this was already imported into WebKit
in bug #168657 but never used yet as the parsing was disabled.
We should enable the use of percentages in parsing
and then import the tests from Blink related to this.
Comment 1 Manuel Rego Casasnovas 2017-04-11 22:58:37 PDT
Created attachment 306891 [details]
Patch
Comment 2 Manuel Rego Casasnovas 2017-04-12 01:40:19 PDT
Created attachment 306897 [details]
Mini change on ChangeLog
Comment 3 Build Bot 2017-04-12 02:45:57 PDT
Comment on attachment 306897 [details]
Mini change on ChangeLog

Attachment 306897 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3522867

New failing tests:
webrtc/negotiatedneeded-event-addStream.html
Comment 4 Build Bot 2017-04-12 02:45:58 PDT
Created attachment 306901 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 5 Build Bot 2017-04-12 04:00:29 PDT
Comment on attachment 306897 [details]
Mini change on ChangeLog

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

New failing tests:
compositing/absolute-inside-out-of-view-fixed.html
Comment 6 Build Bot 2017-04-12 04:00:30 PDT
Created attachment 306906 [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 Sergio Villar Senin 2017-04-18 07:43:00 PDT
Comment on attachment 306897 [details]
Mini change on ChangeLog

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

> LayoutTests/fast/css-grid-layout/grid-gutters-as-percentage.html:15
> +.rows50-50 { grid-template-rows: 50px 50px; }

We haven't used this kind of notation %d-%d for class names in the past. We normally do things like two100Column or things like that.

> LayoutTests/fast/css-grid-layout/grid-gutters-get-set.html:33
> +.gridInvalidColumnGap { grid-column-gap: -webkit-max-content; }

Seems unrelated?

> LayoutTests/fast/css-grid-layout/grid-gutters-get-set.html:44
> +.gridInvalidImplicitGridGap { grid-gap: -webkit-fit-content; }

Ditto.
Comment 8 Manuel Rego Casasnovas 2017-04-18 08:42:55 PDT
Created attachment 307385 [details]
Patch for landing
Comment 9 Manuel Rego Casasnovas 2017-04-18 08:45:33 PDT
Comment on attachment 306897 [details]
Mini change on ChangeLog

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

Thanks for the review.

>> LayoutTests/fast/css-grid-layout/grid-gutters-as-percentage.html:15
>> +.rows50-50 { grid-template-rows: 50px 50px; }
> 
> We haven't used this kind of notation %d-%d for class names in the past. We normally do things like two100Column or things like that.

Ok, changed that.

>> LayoutTests/fast/css-grid-layout/grid-gutters-get-set.html:33
>> +.gridInvalidColumnGap { grid-column-gap: -webkit-max-content; }
> 
> Seems unrelated?

Indeed, a copy&paste issue. :-)
Comment 10 WebKit Commit Bot 2017-04-18 09:27:56 PDT
Comment on attachment 307385 [details]
Patch for landing

Clearing flags on attachment: 307385

Committed r215463: <http://trac.webkit.org/changeset/215463>
Comment 11 WebKit Commit Bot 2017-04-18 09:27:58 PDT
All reviewed patches have been landed.  Closing bug.