Changeset 129136 in webkit


Ignore:
Timestamp:
Sep 20, 2012, 8:05:30 AM (13 years ago)
Author:
jchaffraix@webkit.org
Message:

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.

Location:
trunk/Source/WebCore
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r129122 r129136  
     12012-09-20  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        Remove isStartColumn in the border collapsing code
     4        https://bugs.webkit.org/show_bug.cgi?id=97024
     5
     6        Reviewed by Abhishek Arya.
     7
     8        isStartColumn is embedding the same information as prevCell. As we need to compute it
     9        in most of the cases, we may as well just reuse them.
     10
     11        While touching this code, I cleaned up the code by removing some unneeded checks and renaming
     12        some variables in preparation for bug 79272.
     13
     14        Refactoring covered by existing collapsing borders tests.
     15
     16        * rendering/RenderTableCell.cpp:
     17        (WebCore::RenderTableCell::computeCollapsedStartBorder):
     18        Removed |isStartColumn|.
     19
     20        (WebCore::RenderTableCell::computeCollapsedEndBorder):
     21        Added a comment about why |isEndColumn| is needed. Also cleaned up this code to be
     22        consistent with computeCollapsedStartBorder.
     23
    1242012-09-20  Andrey Adaikin  <aandrey@chromium.org>
    225
  • trunk/Source/WebCore/rendering/RenderTableCell.cpp

    r129078 r129136  
    413413{
    414414    RenderTable* table = this->table();
    415     bool isStartColumn = col() == 0;
    416415
    417416    // For the start border, we need to check, in order of precedence:
     
    422421
    423422    // (2) The end border of the preceding cell.
    424     if (RenderTableCell* prevCell = table->cellBefore(this)) {
    425         CollapsedBorderValue prevCellBorder = CollapsedBorderValue(prevCell->borderAdjoiningCellAfter(this), includeColor ? prevCell->style()->visitedDependentColor(endColorProperty) : Color(), BCELL);
    426         result = chooseBorder(prevCellBorder, result);
    427         if (!result.exists())
    428             return result;
    429     } else if (isStartColumn) {
     423    RenderTableCell* cellBefore = table->cellBefore(this);
     424    if (cellBefore) {
     425        CollapsedBorderValue cellBeforeAdjoiningBorder = CollapsedBorderValue(cellBefore->borderAdjoiningCellAfter(this), includeColor ? cellBefore->style()->visitedDependentColor(endColorProperty) : Color(), BCELL);
     426        // |result| should be the 2nd argument as |cellBefore| should win in case of equality per CSS 2.1 (Border conflict resolution, point 4).
     427        result = chooseBorder(cellBeforeAdjoiningBorder, result);
     428        if (!result.exists())
     429            return result;
     430    } else {
    430431        // (3) Our row's start border.
    431432        result = chooseBorder(result, CollapsedBorderValue(row()->borderAdjoiningStartCell(this), includeColor ? parent()->style()->visitedDependentColor(startColorProperty) : Color(), BROW));
     
    455456   
    456457    // (6) The end border of the preceding column.
    457     if (!isStartColumn) {
     458    if (cellBefore) {
    458459        colElt = table->colElement(col() -1, &startColEdge, &endColEdge);
    459460        if (colElt && endColEdge) {
    460             CollapsedBorderValue endBorder = CollapsedBorderValue(colElt->borderAdjoiningCellAfter(this), includeColor ? colElt->style()->visitedDependentColor(endColorProperty) : Color(), BCOL);
    461             result = chooseBorder(endBorder, result);
     461            CollapsedBorderValue columnBeforeAdjoiningBorder = CollapsedBorderValue(colElt->borderAdjoiningCellAfter(this), includeColor ? colElt->style()->visitedDependentColor(endColorProperty) : Color(), BCOL);
     462            result = chooseBorder(columnBeforeAdjoiningBorder, result);
    462463            if (!result.exists())
    463464                return result;
     
    484485{
    485486    RenderTable* table = this->table();
     487    // Note: We have to use the effective column information instead of whether we have a cell after as a table doesn't
     488    // have to be regular (any row can have less cells than the total cell count).
    486489    bool isEndColumn = table->colToEffCol(col() + colSpan() - 1) == table->numEffCols() - 1;
    487490
     
    494497    // (2) The start border of the following cell.
    495498    if (!isEndColumn) {
    496         RenderTableCell* nextCell = table->cellAfter(this);
    497         if (nextCell && nextCell->style()) {
    498             CollapsedBorderValue startBorder = CollapsedBorderValue(nextCell->borderAdjoiningCellBefore(this), includeColor ? nextCell->style()->visitedDependentColor(startColorProperty) : Color(), BCELL);
    499             result = chooseBorder(result, startBorder);
     499        if (RenderTableCell* cellAfter = table->cellAfter(this)) {
     500            CollapsedBorderValue cellAfterAdjoiningBorder = CollapsedBorderValue(cellAfter->borderAdjoiningCellBefore(this), includeColor ? cellAfter->style()->visitedDependentColor(startColorProperty) : Color(), BCELL);
     501            result = chooseBorder(result, cellAfterAdjoiningBorder);
    500502            if (!result.exists())
    501503                return result;
     
    532534        colElt = table->colElement(col() + colSpan(), &startColEdge, &endColEdge);
    533535        if (colElt && startColEdge) {
    534             CollapsedBorderValue startBorder = CollapsedBorderValue(colElt->borderAdjoiningCellBefore(this), includeColor ? colElt->style()->visitedDependentColor(startColorProperty) : Color(), BCOL);
    535             result = chooseBorder(result, startBorder);
     536            CollapsedBorderValue columnAfterAdjoiningBorder = CollapsedBorderValue(colElt->borderAdjoiningCellBefore(this), includeColor ? colElt->style()->visitedDependentColor(startColorProperty) : Color(), BCOL);
     537            result = chooseBorder(result, columnAfterAdjoiningBorder);
    536538            if (!result.exists())
    537539                return result;
Note: See TracChangeset for help on using the changeset viewer.