[webkit-reviews] review denied: [Bug 19681] borderColor on table elements should be ignored : [Attachment 150864] patch to ignore bordercolor on table when no border is specified

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has denied Kishore Bolisetty
<ksvg.bolisetty at gmail.com>'s request for review:
Bug 19681: borderColor on table elements should be ignored
https://bugs.webkit.org/show_bug.cgi?id=19681

Attachment 150864: patch to ignore bordercolor on table when no border is
specified 
https://bugs.webkit.org/attachment.cgi?id=150864&action=review

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=150864&action=review


AFAICT your change doesn't change anything in how the function behave, yet we
pass your test. Is it possible that this got fixed in the meantime and that you
only need to land your test case?

> Source/WebCore/html/HTMLTableElement.cpp:450
> +    if (m_borderColorAttr && m_borderColorAttr) {

This change doesn't do anything.

> Source/WebCore/html/HTMLTableElement.cpp:455
> +    if (m_borderAttr) {

Per the checks above, this should be an ASSERT as you cannot reach this point
without having m_borderAttr be NULL.

> Source/WebCore/html/HTMLTableElement.cpp:460
> +
> +    return 0;

That case can't be reached as it would mean that !m_borderColorAttr &&
!m_borderAttr (which is checked above).