[webkit-reviews] review canceled: [Bug 76799] display: table-cell does not work properly when applied to img elements : [Attachment 137773] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has canceled Shezan Baig
<shezbaig.wk at gmail.com>'s request for review:
Bug 76799: display: table-cell does not work properly when applied to img
elements
https://bugs.webkit.org/show_bug.cgi?id=76799

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

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


(In reply to comment #5)
> Yes, this affects any elements that override createRenderer and don't return
RenderTableCell when 'display: table-cell' is set.

OK, then I would like some more testing to check that it matches the other
browsers. You don't need to check all cases (more is better though) but at
least 2 or 3. That would give us confidence that this matches other browsers on
more grounds.

> Elements that don't override createRenderer are not affected, because they
will never enter this block of code, because the 'child->isTableCell()' check
that happens at line 91 (not visible in patch) will return true.

Makes sense.

> Source/WebCore/rendering/RenderTableRow.cpp:95
> +	   if (last && last->isAnonymous() && last->isTableCell() &&
!last->isBeforeOrAfterContent() && child->style()->display() != TABLE_CELL) {

Could you add a comment about that? It took me some time to figure out why this
was right.

> LayoutTests/fast/table/img-as-table-cell-expected.html:5
> +    .filler { background-color: green; }
> +    .grey { background-color: lightgrey; }

I wonder if those are needed for the test to work properly. Shouldn't they be
removed as they don't add much or am I missing something?

> LayoutTests/fast/table/img-as-table-cell-expected.html:7
> +<div>There should be 3 rows and 3 columns, like this:</div>

I like to have the bug information included in any test cases (more points if
it's clickable).