New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 346150 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

The height of table cell is not correctly calculated if a rowSpan is specified and the next row has empty cell

Reported by li.alan....@gmail.com, Feb 24 2014

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.107 Safari/537.36

Steps to reproduce the problem:
1. create a page with below content:
<!DOCTYPE html>
<html>
<body>
    <table id="container" cellspacing="0"  cellpadding="0">
		<tbody>
            <tr>
                <td>This is a test</td>
            </tr>
            <tr>
                <td>This is a test</td>
            </tr>
            <tr>
                <td>This is a test</td>
            </tr>
            <tr>
                <td>This is a test</td>
            </tr>            
            <tr>
				<td id="img_cell" rowspan="2"> <!-- rowspan causes the problem -->
                    <img border="0" src="https://www.google.com.sg/images/srpr/logo11w.png" alt="Logo" title="Logo" style="display:block;"/>
                </td>
			</tr>
            <tr id="row_under_img">
                <td></td>
            </tr>
		</tbody>
	</table>
</body>
</html>
or visit: http://jsfiddle.net/mumuli/85dQZ/9/

2. You may find the "Google" logo is wrongly positioned at the left up corner.

The height of the cell("img_cell") which contains the logo is 0px.

What is the expected behavior?
The "Google" logo should be rendered after the last "This is a test"

What went wrong?
The calculation of the height for the cell which has rowSpan defined is wrong.

Did this work before? N/A 

Chrome version: 32.0.1700.107  Channel: n/a
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: Shockwave Flash 12.0 r0

1. If the "cellpadding" is removed from the table, the issue is gone.
2. If the rowspan is removed from the cell, the issue is gone.
3. If the empty row is removed, the issue is gone.
 
Forgot one thing. If the cell under the image cell has content, the issue is gone. 

       <tr id="row_under_img">
          <td>Any Text</td>
       </tr>

Comment 2 by tkent@chromium.org, Feb 24 2014

Cc: jchaffraix@chromium.org
Labels: -Cr-UI Cr-Blink-Rendering
Cc: a.suc...@samsung.com ashej...@chromium.org
Labels: -Pri-2 -Type-Bug Pri-1 Type-Bug-Regression M-35 ReleaseBlock-Beta
Status: Assigned
Able to reproduce the above issue on chrome latest stable & canary version & below is the regression info:

You are probably looking for a change made after 158479 (known good), but no lat
er than 158489 (first known bad).
BLINK CHANGELOG URL:
  http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog_blink.html?ur
l=/trunk&range=158489%3A158479.

Suspecting-r158484? @a.suchit@samsung.com: Would you mind checking the above issue & see if it's related. If it's not then do assign it to appropriate developer. Not able to assign it to "a.suchit@samsung.com", hence ccing the developer.

Thank you!


It will fix with patch https://codereview.chromium.org/47923009/
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 3 2014

The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=168326

------------------------------------------------------------------------
r168326 | a.suchit@samsung.com | 2014-03-03T19:59:02.586251Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderTableSection.cpp?r1=168326&r2=168325&pathrev=168326
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/table/table-rowspan-cell-with-empty-cell.html?r1=168326&r2=168325&pathrev=168326
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/linux/tables/mozilla/bugs/bug220536-expected.png?r1=168326&r2=168325&pathrev=168326
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=168326&r2=168325&pathrev=168326
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/linux/fast/table/giantRowspan2-expected.png?r1=168326&r2=168325&pathrev=168326
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderTableSection.h?r1=168326&r2=168325&pathrev=168326
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/table/table-rowspan-cell-with-empty-cell-expected.txt?r1=168326&r2=168325&pathrev=168326
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/win/tables/mozilla/bugs/bug220536-expected.txt?r1=168326&r2=168325&pathrev=168326
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/win/fast/table/giantRowspan2-expected.txt?r1=168326&r2=168325&pathrev=168326

Table rows are incorrectly collapsed in case of hidden cells and rowspans.

When all rows in spanning cell are having only row spanning cells with empty
cell than None of the rows in spanning cell was not getting any height.

Total height of the rows in spanning cell should have at least spanning cell
height so apply spanning cell height in the last row of the spanning cells
if all rows in spanning cell are have zero height.
The total height of the rows in a spanning cell should be at least the height of
the spanning cell's itself because content should fit in the cell. This change
adds the remaining height to the last row because every row is belong to this cell only.

R=jchaffraix@chromium.org
BUG= 258420 , 330564 , 341366 , 346150 

Review URL: https://codereview.chromium.org/47923009
------------------------------------------------------------------------

Comment 6 by kareng@google.com, Mar 6 2014

Owner: jchaffraix@chromium.org
li.alan.lin is it fixed now?
How can I get the fix to verify? 

Verification will be simple. Just checking the rendering for below linkage.
http://jsfiddle.net/mumuli/85dQZ/9/
Status: Fixed
This have fixed.
Cc: karen@chromium.org lafo...@chromium.org matthewyuan@chromium.org dxie@chromium.org
Labels: Merge-TBD
Just checking is there any merge required here, hence adding 'Merge-TBD' label.

Thank you.
Labels: -Merge-TBD
M35 has the fix. It's too late for M34 considering that it's a fairly involved change.
Labels: -Cr-Blink-Rendering Cr-Blink-Layout
Migrate from Cr-Blink-Rendering to Cr-Blink-Layout
Migrate from Cr-Blink-Rendering to Cr-Blink-Layout

Sign in to add a comment