Table conent is rendered over the border when colspan present with rowspan.

Table conent is rendered over the border when colspan present with rowspan.

Some height is given to all row with only spanning cells even it is not required.

If row spanning cell content is flowing out of cell boundries then only
row with only spanning cells gets one part of spanning cell's height.

Both functions isHeightNeededForRowHavingOnlySpanningCells() and
rowHasOnlySpanningCells() are different. logic is same in both but resultant
from both based on different criteria.
rowHasOnlySpanningCells() returns true when only spanning cells present in row.
isHeightNeededForRowHavingOnlySpanningCells() returns true when height of one or more
spanning cells in row is not having expected height.

Example 1:
<tr><td rowspan="2">r0c0</td></tr><tr><td>r1c1</td></tr>
Example 2:
<tr><td rowspan="3">r0c0</td><td>r0c1</td></tr><tr><td>r1c1</td></tr>
In these example, first row in first example and third row in second example
always should have 0 height.
If rowHasOnlySpanningCells() condition is not there then first row will get
height when isHeightNeededForRowHavingOnlySpanningCells() will return true.

R=jchaffraix@chromium.org
BUG=295500
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=158175

[Julien - ping for review, 2013-09-20 18:34:52]

https://codereview.chromium.org/23503083/diff/1/Source/core/rendering/RenderT...
File Source/core/rendering/RenderTableSection.cpp (right):

https://codereview.chromium.org/23503083/diff/1/Source/core/rendering/RenderT...
Source/core/rendering/RenderTableSection.cpp:451: }
This really looks like it should be folded into rowHasOnlySpanningCells, it
looks weird to now have 2 independent conditions for this case.

https://codereview.chromium.org/23503083/diff/1/Source/core/rendering/RenderT...
Source/core/rendering/RenderTableSection.cpp:485: if
(!spanningRowsHeight.rowHeight[row] && rowHasOnlySpanningCells(actualRow) &&
isHeightNeededForRowHavingOnlySpanningCells(actualRow)) {
Is the !spanningRowsHeight.rowHeight[row] still needed now that we actually
compare the total spanned height against the cell's height?

[suchit.agrawal, 2013-09-20 22:12:00]

https://codereview.chromium.org/23503083/diff/1/Source/core/rendering/RenderT...
File Source/core/rendering/RenderTableSection.cpp (right):

https://codereview.chromium.org/23503083/diff/1/Source/core/rendering/RenderT...
Source/core/rendering/RenderTableSection.cpp:485: if
(!spanningRowsHeight.rowHeight[row] && rowHasOnlySpanningCells(actualRow) &&
isHeightNeededForRowHavingOnlySpanningCells(actualRow)) {
On 2013/09/20 18:34:53, Julien Chaffraix wrote:
> Is the !spanningRowsHeight.rowHeight[row] still needed now that we actually
> compare the total spanned height against the cell's height?

Yes !spanningRowsHeight.rowHeight[row] is needed because row will get this type
of height only when spanningRowsHeight.rowHeight[row] is 0 and spanning cell's
height is less then expected.
Spanning cell's height cann't be compared with any one row because it spans is
multiple rows.

[Julien - ping for review, 2013-09-21 00:52:13]

We talked about the patch on IRC and Suchit explained why the 2 concepts are
different and required. Based on this discussion, lgtm.

Please fix the description to include some examples and why it's helpful to keep
the concepts separate.

[commit-bot: I haz the power, 2013-09-21 03:15:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.suchit@samsung.com/23503083/1

[commit-bot: I haz the power, 2013-09-21 03:16:01]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 1324.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
5e94dab1d2246036ba0f41b317f1f9920d6105c7..95609baaedf445ffe7169cf0000ffe27fe947cb7
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -1324,3 +1324,10 @@ crbug.com/51854 [ Win ]
http/tests/storage/callbacks-are-called-in-correct-conte
 
 crbug.com/295011 [ Win ] http/tests/cache/subresource-expiration-1.html [
Timeout Pass ]
 crbug.com/295011 [ Win ] http/tests/cache/subresource-expiration-2.html [
Timeout Pass ]
+
+#Below test case need to rebaseline after crbug.com/295500
+crbug.com/295500 fast/table/007.html [ NeedsRebaseline ]
+crbug.com/295500 fast/table/colspanMinWidth-vertical.html [ NeedsRebaseline ]
+crbug.com/295500 fast/table/colspanMinWidth.html [ NeedsRebaseline ]
+crbug.com/295500 fast/table/spanOverlapRepaint.html [ NeedsRebaseline ]
+crbug.com/295500 tables/mozilla/bugs/bug6304.html [ NeedsRebaseline ]

[commit-bot: I haz the power, 2013-09-21 03:16:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.suchit@samsung.com/23503083/1

[commit-bot: I haz the power, 2013-09-21 03:17:00]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 1324.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
5e94dab1d2246036ba0f41b317f1f9920d6105c7..95609baaedf445ffe7169cf0000ffe27fe947cb7
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -1324,3 +1324,10 @@ crbug.com/51854 [ Win ]
http/tests/storage/callbacks-are-called-in-correct-conte
 
 crbug.com/295011 [ Win ] http/tests/cache/subresource-expiration-1.html [
Timeout Pass ]
 crbug.com/295011 [ Win ] http/tests/cache/subresource-expiration-2.html [
Timeout Pass ]
+
+#Below test case need to rebaseline after crbug.com/295500
+crbug.com/295500 fast/table/007.html [ NeedsRebaseline ]
+crbug.com/295500 fast/table/colspanMinWidth-vertical.html [ NeedsRebaseline ]
+crbug.com/295500 fast/table/colspanMinWidth.html [ NeedsRebaseline ]
+crbug.com/295500 fast/table/spanOverlapRepaint.html [ NeedsRebaseline ]
+crbug.com/295500 tables/mozilla/bugs/bug6304.html [ NeedsRebaseline ]

[commit-bot: I haz the power, 2013-09-23 06:35:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.suchit@samsung.com/23503083/16001

[commit-bot: I haz the power, 2013-09-23 07:28:55]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_blink_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-09-23 09:25:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.suchit@samsung.com/23503083/33001

[commit-bot: I haz the power, 2013-09-23 09:25:06]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 1378.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
cc72adcdcc60b98560e73576490855f9dcba2488..933698d566315afc53095bb2716619ddea7f8c81
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -1378,3 +1378,10 @@ crbug.com/292740
http/tests/security/xssAuditor/xss-filter-bypass-big5.html [ Ne
 crbug.com/292740 http/tests/security/xssAuditor/xss-filter-bypass-sjis.html [
NeedsManualRebaseline ]
 crbug.com/292740 http/tests/xmlhttprequest/origin-exact-matching.html [
NeedsManualRebaseline Timeout ]
 #crbug.com/292740 http/tests/xmlhttprequest/origin-whitelisting-https.html [
NeedsManualRebaseline ]
+
+#Below test case need to rebaseline after crbug.com/295500
+crbug.com/295500 fast/table/007.html [ NeedsRebaseline ]
+crbug.com/295500 fast/table/colspanMinWidth-vertical.html [ NeedsRebaseline ]
+crbug.com/295500 fast/table/colspanMinWidth.html [ NeedsRebaseline ]
+crbug.com/295500 fast/table/spanOverlapRepaint.html [ NeedsRebaseline ]
+crbug.com/295500 tables/mozilla/bugs/bug6304.html [ NeedsRebaseline ]

[commit-bot: I haz the power, 2013-09-23 09:31:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.suchit@samsung.com/23503083/42001

[commit-bot: I haz the power, 2013-09-23 10:43:41]

Change committed as 158175