Bug 101462 – CSS3 Multicolumn: column-span should accept value 'none' (instead of '1')

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Chris Dumez]

In http://www.w3.org/TR/2009/CR-css3-multicol-20091217/#column-span:
column-span was accepting values: 1 | all

but in http://www.w3.org/TR/2011/CR-css3-multicol-20110412/#column-span:
possibles values are now: none | all

There was no change in meaning but the value '1' was renamed to 'none'. We should update the WebKit implementation accordingly.

[Chris Dumez]

Created attachment 172790 [details]
Patch

[Build Bot]

Comment on attachment 172790 [details]
Patch

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

New failing tests:
fast/multicol/span/span-as-immediate-child-property-removal.html

[Chris Dumez]

Created attachment 172819 [details]
Patch

Forgot to update the following test case:
fast/multicol/span/span-as-immediate-child-property-removal.html

[Build Bot]

Comment on attachment 172819 [details]
Patch

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

New failing tests:
fast/multicol/span/span-as-immediate-child-property-removal.html

[Chris Dumez]

Created attachment 172863 [details]
Patch

Second attempt to make mac-ews happy.

[Chris Dumez]

Any feedback on this patch please?

[Kentaro Hara]

Comment on attachment 172863 [details]
Patch

The implementation looks good but I'm not familiar with CSS stuff. CSS experts should look.

[Chris Dumez]

Hopefully, people are back from Thanksgiving holiday now :) Could someone please review the patch?

Is there anyone else I need to CC?

[Dave Hyatt]

Comment on attachment 172863 [details]
Patch

This will cause a compatibility issue. For the -webkit- prefix, we need to continue to support the value of 1. If you want to add, none and make it the default, I think that should be safe, but the explicit value of 1 needs to continue to work. When we implement unprefixed column properties, we can drop support for 1 at that time.

[Chris Dumez]

Created attachment 176334 [details]
Patch

Take Dave Hyatt's feedback into consideration. This new patch keeps supporting 1 value for compatibility and maps it to none.

[Dave Hyatt]

Comment on attachment 176334 [details]
Patch

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

Looking good. Just some minor suggestions now.

> LayoutTests/fast/multicol/span/span-as-immediate-child-property-removal.html:135
> -    spanners[i].style.webkitColumnSpan = 1;
> +    spanners[i].style.webkitColumnSpan = 'none';

You don't have to fix up this test now. I would suggest reverting this change so that we keep testing 1 as a possible value here.

> Source/WebCore/css/CSSParser.cpp:2524
> +    case CSSPropertyWebkitColumnSpan: // none | all | 1

Maybe mention that 1 will be removed when we drop the prefix. Something like:

case CSSPropertyWebkitColumnSpan: // none | all | 1 (will be dropped in the unprefixed property)

[Chris Dumez]

Created attachment 176339 [details]
Patch

Take Dave Hyatt's feedback into consideration.

[Dave Hyatt]

Comment on attachment 176339 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 176339 [details]
Patch

Clearing flags on attachment: 176339

Committed r136053: <http://trac.webkit.org/changeset/136053>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.