[webkit-reviews] review granted: [Bug 173049] Align <col span>/<colgroup span> limits with the latest HTML specification : [Attachment 312227] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 173049: Align <col span>/<colgroup span> limits with the latest HTML
specification
https://bugs.webkit.org/show_bug.cgi?id=173049

Attachment 312227: Patch

https://bugs.webkit.org/attachment.cgi?id=312227&action=review




--- Comment #3 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 312227
  --> https://bugs.webkit.org/attachment.cgi?id=312227
Patch

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

> Source/WebCore/ChangeLog:8
> +	   Align<col span>/<colgroup span> limits with the latest HTML
specification after:

Missing a space after "Align".

> Source/WebCore/html/HTMLTableColElement.cpp:39
> +static const unsigned defaultSpan { 1 };
> +static const unsigned minSpan { 1 };
> +static const unsigned maxSpan { 1000 };

The keyword static is unnecessary. In C++, const has internal linkage in file
scope.

Do we use C++ brace initialization for POD constants?

> Source/WebCore/html/HTMLTableColElement.cpp:99
> +    setUnsignedIntegralAttribute(spanAttr, limitToOnlyHTMLNonNegative(span,
defaultSpan));

Please add a remark to the ChangeLog about this change.

It seems weird that setting the span programmatically has different behavior
than parsing the HTML span attribute.