Modified: trunk/Source/WebCore/ChangeLog (129135 => 129136)
--- trunk/Source/WebCore/ChangeLog 2012-09-20 14:50:31 UTC (rev 129135)
+++ trunk/Source/WebCore/ChangeLog 2012-09-20 15:05:30 UTC (rev 129136)
@@ -1,3 +1,26 @@
+2012-09-20 Julien Chaffraix <[email protected]>
+
+ Remove isStartColumn in the border collapsing code
+ https://bugs.webkit.org/show_bug.cgi?id=97024
+
+ Reviewed by Abhishek Arya.
+
+ isStartColumn is embedding the same information as prevCell. As we need to compute it
+ in most of the cases, we may as well just reuse them.
+
+ While touching this code, I cleaned up the code by removing some unneeded checks and renaming
+ some variables in preparation for bug 79272.
+
+ Refactoring covered by existing collapsing borders tests.
+
+ * rendering/RenderTableCell.cpp:
+ (WebCore::RenderTableCell::computeCollapsedStartBorder):
+ Removed |isStartColumn|.
+
+ (WebCore::RenderTableCell::computeCollapsedEndBorder):
+ Added a comment about why |isEndColumn| is needed. Also cleaned up this code to be
+ consistent with computeCollapsedStartBorder.
+
2012-09-20 Andrey Adaikin <[email protected]>
Web Inspector: setPropertyValue does not work for non-finite numbers
Modified: trunk/Source/WebCore/rendering/RenderTableCell.cpp (129135 => 129136)
--- trunk/Source/WebCore/rendering/RenderTableCell.cpp 2012-09-20 14:50:31 UTC (rev 129135)
+++ trunk/Source/WebCore/rendering/RenderTableCell.cpp 2012-09-20 15:05:30 UTC (rev 129136)
@@ -412,7 +412,6 @@
CollapsedBorderValue RenderTableCell::computeCollapsedStartBorder(IncludeBorderColorOrNot includeColor) const
{
RenderTable* table = this->table();
- bool isStartColumn = col() == 0;
// For the start border, we need to check, in order of precedence:
// (1) Our start border.
@@ -421,12 +420,14 @@
CollapsedBorderValue result(style()->borderStart(), includeColor ? style()->visitedDependentColor(startColorProperty) : Color(), BCELL);
// (2) The end border of the preceding cell.
- if (RenderTableCell* prevCell = table->cellBefore(this)) {
- CollapsedBorderValue prevCellBorder = CollapsedBorderValue(prevCell->borderAdjoiningCellAfter(this), includeColor ? prevCell->style()->visitedDependentColor(endColorProperty) : Color(), BCELL);
- result = chooseBorder(prevCellBorder, result);
+ RenderTableCell* cellBefore = table->cellBefore(this);
+ if (cellBefore) {
+ CollapsedBorderValue cellBeforeAdjoiningBorder = CollapsedBorderValue(cellBefore->borderAdjoiningCellAfter(this), includeColor ? cellBefore->style()->visitedDependentColor(endColorProperty) : Color(), BCELL);
+ // |result| should be the 2nd argument as |cellBefore| should win in case of equality per CSS 2.1 (Border conflict resolution, point 4).
+ result = chooseBorder(cellBeforeAdjoiningBorder, result);
if (!result.exists())
return result;
- } else if (isStartColumn) {
+ } else {
// (3) Our row's start border.
result = chooseBorder(result, CollapsedBorderValue(row()->borderAdjoiningStartCell(this), includeColor ? parent()->style()->visitedDependentColor(startColorProperty) : Color(), BROW));
if (!result.exists())
@@ -454,11 +455,11 @@
}
// (6) The end border of the preceding column.
- if (!isStartColumn) {
+ if (cellBefore) {
colElt = table->colElement(col() -1, &startColEdge, &endColEdge);
if (colElt && endColEdge) {
- CollapsedBorderValue endBorder = CollapsedBorderValue(colElt->borderAdjoiningCellAfter(this), includeColor ? colElt->style()->visitedDependentColor(endColorProperty) : Color(), BCOL);
- result = chooseBorder(endBorder, result);
+ CollapsedBorderValue columnBeforeAdjoiningBorder = CollapsedBorderValue(colElt->borderAdjoiningCellAfter(this), includeColor ? colElt->style()->visitedDependentColor(endColorProperty) : Color(), BCOL);
+ result = chooseBorder(columnBeforeAdjoiningBorder, result);
if (!result.exists())
return result;
}
@@ -483,6 +484,8 @@
CollapsedBorderValue RenderTableCell::computeCollapsedEndBorder(IncludeBorderColorOrNot includeColor) const
{
RenderTable* table = this->table();
+ // Note: We have to use the effective column information instead of whether we have a cell after as a table doesn't
+ // have to be regular (any row can have less cells than the total cell count).
bool isEndColumn = table->colToEffCol(col() + colSpan() - 1) == table->numEffCols() - 1;
// For end border, we need to check, in order of precedence:
@@ -493,10 +496,9 @@
// (2) The start border of the following cell.
if (!isEndColumn) {
- RenderTableCell* nextCell = table->cellAfter(this);
- if (nextCell && nextCell->style()) {
- CollapsedBorderValue startBorder = CollapsedBorderValue(nextCell->borderAdjoiningCellBefore(this), includeColor ? nextCell->style()->visitedDependentColor(startColorProperty) : Color(), BCELL);
- result = chooseBorder(result, startBorder);
+ if (RenderTableCell* cellAfter = table->cellAfter(this)) {
+ CollapsedBorderValue cellAfterAdjoiningBorder = CollapsedBorderValue(cellAfter->borderAdjoiningCellBefore(this), includeColor ? cellAfter->style()->visitedDependentColor(startColorProperty) : Color(), BCELL);
+ result = chooseBorder(result, cellAfterAdjoiningBorder);
if (!result.exists())
return result;
}
@@ -531,8 +533,8 @@
if (!isEndColumn) {
colElt = table->colElement(col() + colSpan(), &startColEdge, &endColEdge);
if (colElt && startColEdge) {
- CollapsedBorderValue startBorder = CollapsedBorderValue(colElt->borderAdjoiningCellBefore(this), includeColor ? colElt->style()->visitedDependentColor(startColorProperty) : Color(), BCOL);
- result = chooseBorder(result, startBorder);
+ CollapsedBorderValue columnAfterAdjoiningBorder = CollapsedBorderValue(colElt->borderAdjoiningCellBefore(this), includeColor ? colElt->style()->visitedDependentColor(startColorProperty) : Color(), BCOL);
+ result = chooseBorder(result, columnAfterAdjoiningBorder);
if (!result.exists())
return result;
}
