[webkit-reviews] review denied: [Bug 90068] Setting table layout to fixed causes incorrect cell width calculations : [Attachment 155067] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has denied Pravin D
<pravind.2k4 at gmail.com>'s request for review:
Bug 90068: Setting table layout to fixed causes incorrect cell width
calculations
https://bugs.webkit.org/show_bug.cgi?id=90068

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

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


> Source/WebCore/rendering/RenderTableSection.cpp:272
> +	   if (m_cCol == nCols) {
> +	       if (cSpan > columns[m_cCol].span)
> +		   columns[m_cCol].span = cSpan;
> +	       break;
> +	   }

I don't think it is the right fix: RenderTable::splitColumn updates all
sections' representation. Here you override the columns[m_cCol].span *after*
splitting which will cause some sections' representation to drift from the
table's and will be the cause of super nasty bugs. You could probably see the
bug if you had multiple sections (which is a good test to add). I would support
adding some ASSERT after splitting / adding columns that all our
representations are in sync.

If anything RenderTable::splitColumn should be the one handling that properly
so that we propagate the right information to all sections. Also why does auto
table layout works correctly whereas fixed table layout doesn't?