[webkit-reviews] review denied: [Bug 101462] CSS3 Multicolumn: column-span should accept value 'none' (instead of '1') : [Attachment 176334] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Christophe Dumez
<christophe.dumez at intel.com>'s request for review:
Bug 101462: CSS3 Multicolumn: column-span should accept value 'none' (instead
of '1')
https://bugs.webkit.org/show_bug.cgi?id=101462

Attachment 176334: Patch
https://bugs.webkit.org/attachment.cgi?id=176334&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
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:13
5
> -    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)