[webkit-reviews] review canceled: [Bug 83978] CSS 2.1 failure: table-columns-example-001 fails : [Attachment 137949] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Apr 19 16:55:03 PDT 2012
Julien Chaffraix <jchaffraix at webkit.org> has canceled Robert Hogan
<robert at webkit.org>'s request for review:
Bug 83978: CSS 2.1 failure: table-columns-example-001 fails
https://bugs.webkit.org/show_bug.cgi?id=83978
Attachment 137949: Patch
https://bugs.webkit.org/attachment.cgi?id=137949&action=review
------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=137949&action=review
The code is fine. But talking the whole ref tests with another reviewer, we
have the same feeling that the tests you are adding should be ref-tests. For a
few reasons:
* it is simple to have a reference (using the border-collapse: separate
equivalent of what you expect)
* you don't care about the tree dump
* it's smaller and we get less potential baselines.
> Source/WebCore/html/HTMLTableElement.cpp:435
> + if (m_rulesAttr) {
I prefer to do the full comparison: m_rulesAttr != UnsetRules.
> LayoutTests/css2.1/20110323/table-columns-example-001.htm:23
> + height: 2em;
> + width: 2em;
Again, why do those needs to be ems? I fail to see the need but it may just be
me being thick.
More information about the webkit-reviews
mailing list