[CSS Grid Layout] Test coverage for column-* properties.

[CSS Grid Layout] Test coverage for column-* properties.

According to the spec: all of the column-* properties in the Multicol module
have no effect on a grid container.

This was already working as expected but we were missing test for this.

TEST:fast/css-grid-layout/column-property-should-not-apply-on-grid-container.html

BUG=395788
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180814

[Sunil Ratnu, 2014-08-20 14:59:14]

Please review this. Thanks!

[Julien - ping for review, 2014-08-20 18:14:48]

lgtm

https://codereview.chromium.org/493683002/diff/1/LayoutTests/fast/css-grid-la...
File
LayoutTests/fast/css-grid-layout/column-property-should-not-apply-on-grid-container.html
(right):

https://codereview.chromium.org/493683002/diff/1/LayoutTests/fast/css-grid-la...
LayoutTests/fast/css-grid-layout/column-property-should-not-apply-on-grid-container.html:15:
column-width: 1em;
Let's get rid of the implicit assumption that the grid width is the viewport's
by adding an explicit width here. I would also just set the column-count as it's
easier to see what's going on (and avoids having a lot of columns).

My take would be:
width: 20em;
column-count:2;

(which would nicely split the content into 2 columns, thus showing the issue a
lot more than a lot of columns).

[Sunil Ratnu, 2014-08-21 05:11:17]

https://codereview.chromium.org/493683002/diff/1/LayoutTests/fast/css-grid-la...
File
LayoutTests/fast/css-grid-layout/column-property-should-not-apply-on-grid-container.html
(right):

https://codereview.chromium.org/493683002/diff/1/LayoutTests/fast/css-grid-la...
LayoutTests/fast/css-grid-layout/column-property-should-not-apply-on-grid-container.html:15:
column-width: 1em;
Thanks Julien for the review. I have made the suggested changes.
Also,
1. I have removed column-width property as column-count itself is sufficient
enough.
2. Added column-gap so that this CL tests more than one column-* property as
mentioned in the title.
Please have an another look before commiting.
On 2014/08/20 18:14:48, Julien Chaffraix - PST wrote:
> Let's get rid of the implicit assumption that the grid width is the viewport's
> by adding an explicit width here. I would also just set the column-count as
it's
> easier to see what's going on (and avoids having a lot of columns).
> 
> My take would be:
> width: 20em;
> column-count:2;
> 
> (which would nicely split the content into 2 columns, thus showing the issue a
> lot more than a lot of columns).

[Julien - ping for review, 2014-08-21 15:23:49]

lgtm with more content to show the bug.

https://codereview.chromium.org/493683002/diff/20001/LayoutTests/fast/css-gri...
File
LayoutTests/fast/css-grid-layout/column-property-should-not-apply-on-grid-container.html
(right):

https://codereview.chromium.org/493683002/diff/20001/LayoutTests/fast/css-gri...
LayoutTests/fast/css-grid-layout/column-property-should-not-apply-on-grid-container.html:21:
AAAAAAAAAA BBBBBBBBBB
I would have left the content untouched. The reason is that if you don't have
enough content to fill a second line in your multi-column, we wouldn't see the
difference visually.

[Sunil Ratnu, 2014-08-22 04:57:30]

https://codereview.chromium.org/493683002/diff/20001/LayoutTests/fast/css-gri...
File
LayoutTests/fast/css-grid-layout/column-property-should-not-apply-on-grid-container.html
(right):

https://codereview.chromium.org/493683002/diff/20001/LayoutTests/fast/css-gri...
LayoutTests/fast/css-grid-layout/column-property-should-not-apply-on-grid-container.html:21:
AAAAAAAAAA BBBBBBBBBB
On 2014/08/21 15:23:48, Julien Chaffraix - PST wrote:
> I would have left the content untouched. The reason is that if you don't have
> enough content to fill a second line in your multi-column, we wouldn't see the
> difference visually.

Done.

[Sunil Ratnu, 2014-08-22 04:57:57]

The CQ bit was checked by sunil.ratnu@samsung.com

[commit-bot: I haz the power, 2014-08-22 04:58:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sunil.ratnu@samsung.com/493683002/40001

[commit-bot: I haz the power, 2014-08-22 05:54:55]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)

[commit-bot: I haz the power, 2014-08-22 05:58:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-22 05:58:23]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)

[Sunil Ratnu, 2014-08-22 06:10:09]

The CQ bit was checked by sunil.ratnu@samsung.com

[commit-bot: I haz the power, 2014-08-22 06:10:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sunil.ratnu@samsung.com/493683002/40001

[commit-bot: I haz the power, 2014-08-22 06:13:16]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)

[commit-bot: I haz the power, 2014-08-22 06:15:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-22 06:15:45]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)

[Sunil Ratnu, 2014-08-22 07:48:04]

The CQ bit was checked by sunil.ratnu@samsung.com

[commit-bot: I haz the power, 2014-08-22 07:49:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sunil.ratnu@samsung.com/493683002/40001

[commit-bot: I haz the power, 2014-08-22 07:52:00]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)

[commit-bot: I haz the power, 2014-08-22 07:56:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-22 07:56:10]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)

[Sunil Ratnu, 2014-08-22 10:33:16]

The CQ bit was checked by sunil.ratnu@samsung.com

[commit-bot: I haz the power, 2014-08-22 10:34:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sunil.ratnu@samsung.com/493683002/40001

[commit-bot: I haz the power, 2014-08-22 10:37:06]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)

[commit-bot: I haz the power, 2014-08-22 10:39:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-22 10:39:44]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)

[Sunil Ratnu, 2014-08-23 04:51:45]

The CQ bit was checked by sunil.ratnu@samsung.com

[commit-bot: I haz the power, 2014-08-23 04:52:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sunil.ratnu@samsung.com/493683002/40001

[commit-bot: I haz the power, 2014-08-24 04:53:50]

Committed patchset #3 (40001) as 180814