Changeset 119594 in webkit


Ignore:
Timestamp:
Jun 6, 2012 9:07:16 AM (12 years ago)
Author:
jchaffraix@webkit.org
Message:

Add support for direction on table row group with collapsing borders
​https://bugs.webkit.org/show_bug.cgi?id=87900

Reviewed by Ojan Vafai.

Source/WebCore:

Tests: fast/table/border-collapsing/first-cell-left-border-hidden-table-ltr-section-rtl.html

fast/table/border-collapsing/last-cell-left-border-hidden-table-ltr-section-rtl.html
fast/table/border-collapsing/left-border-table-ltr-section-rtl.html
fast/table/border-collapsing/left-border-table-rtl-section-ltr.html
fast/table/border-collapsing/left-border-table-rtl-section-rtl.html
fast/table/border-collapsing/left-border-vertical-lr-table-ltr-section-rtl.html
fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-ltr.html
fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-rtl.html
fast/table/border-collapsing/rtl-table-left-border-hidden.html
fast/table/border-collapsing/top-border-vertical-rl-table-ltr-section-rtl.html
fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-ltr.html
fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-rtl.html
fast/table/table-ltr-section-rtl.html
fast/table/table-rtl-section-ltr.html
fast/table/table-rtl-section-rtl.html

This change enables proper support for direction on table row group.
The current code would allow people to set direction on the row group but would still
use the table's code for directionality checks (with surprising consequences).

The main change involve swapping end / start borders in case of mixed directionality
as those 2 are not in sync.

Example: <table dir="ltr"><tbody dir="rtl"><td id="cell0"></td><td id="cell1"></td></tbody></table>

Visually:
Table: Start ---------------------> End
Tbody: End <---------------------- Start
Cell: | #cell0 | #cell1 |

In this example, the end table border should be compared with the tbody's start border
and the last cell's (in DOM order) start border.

  • rendering/RenderTable.cpp:

(WebCore::RenderTable::tableStartBorderAdjoiningCell):
(WebCore::RenderTable::tableEndBorderAdjoiningCell):
Same as the other adjoining functions.

  • rendering/RenderTable.h:

(WebCore::RenderTable::lastColumnIndex):
Helper function to get the last column index.

  • rendering/RenderTableCell.cpp:

(WebCore::RenderTableCell::computeCollapsedStartBorder):
(WebCore::RenderTableCell::computeCollapsedEndBorder):
Updated to call the table's adjoining border helpers.

  • rendering/RenderTableCell.h:

(WebCore::RenderTableCell::styleForCellFlow):
Updated to use the table row group's style now that we properly support it.

(WebCore::RenderTableCell::isFirstOrLastCellInRow):
Debug only helper to make sure we don't call the border adjoining function
on non-terminal cells.

(WebCore::RenderTableCell::borderAdjoiningTableStart):
(WebCore::RenderTableCell::borderAdjoiningTableEnd):

  • rendering/RenderTableRow.h:

(WebCore::RenderTableRow::borderAdjoiningTableStart):
(WebCore::RenderTableRow::borderAdjoiningTableEnd):

  • rendering/RenderTableSection.cpp:

(WebCore::RenderTableSection::firstRowCellAdjoiningTableStart):
(WebCore::RenderTableSection::firstRowCellAdjoiningTableEnd):
Updated the previous function to account for mixed directionality.

(WebCore::RenderTableSection::layoutRows):
Move some of the code to setLogicalPositionForCell to match RenderBlock.

(WebCore::RenderTableSection::setLogicalPositionForCell):
Switched direction checks to styleForCellFlow (this doesn't change anything as we
were already properly flipping already but ensure better).

  • rendering/RenderTableSection.h:

(WebCore::RenderTableSection::hasSameDirectionAsTable):
Added this helper to know if we have a mixed direction.

(WebCore::RenderTableSection::borderAdjoiningTableStart):
(WebCore::RenderTableSection::borderAdjoiningTableEnd):
Updated to account for mixed directionality.

LayoutTests:

  • fast/table/border-collapsing/first-cell-left-border-hidden-table-ltr-section-rtl-expected.html: Added.
  • fast/table/border-collapsing/first-cell-left-border-hidden-table-ltr-section-rtl.html: Added.
  • fast/table/border-collapsing/last-cell-left-border-hidden-table-ltr-section-rtl-expected.html: Added.
  • fast/table/border-collapsing/last-cell-left-border-hidden-table-ltr-section-rtl.html: Added.
  • fast/table/border-collapsing/left-border-table-ltr-section-rtl-expected.html: Added.
  • fast/table/border-collapsing/left-border-table-ltr-section-rtl.html: Added.
  • fast/table/border-collapsing/left-border-table-rtl-section-ltr-expected.html: Added.
  • fast/table/border-collapsing/left-border-table-rtl-section-ltr.html: Added.
  • fast/table/border-collapsing/left-border-table-rtl-section-rtl-expected.html: Added.
  • fast/table/border-collapsing/left-border-table-rtl-section-rtl.html: Added.
  • fast/table/border-collapsing/left-border-vertical-lr-table-ltr-section-rtl-expected.html: Added.
  • fast/table/border-collapsing/left-border-vertical-lr-table-ltr-section-rtl.html: Added.
  • fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-ltr-expected.html: Added.
  • fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-ltr.html: Added.
  • fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-rtl-expected.html: Added.
  • fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-rtl.html: Added.
  • fast/table/border-collapsing/rtl-table-left-border-hidden-expected.html: Added.
  • fast/table/border-collapsing/rtl-table-left-border-hidden.html: Added.
  • fast/table/border-collapsing/top-border-vertical-rl-table-ltr-section-rtl-expected.html: Added.
  • fast/table/border-collapsing/top-border-vertical-rl-table-ltr-section-rtl.html: Added.
  • fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-ltr-expected.html: Added.
  • fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-ltr.html: Added.
  • fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-rtl-expected.html: Added.
  • fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-rtl.html: Added.

Most of those are test are a variation of direction on table and row group along with disabling some
borders (either using 'hidden' or just disabling it by setting it to 0px).

  • fast/table/table-ltr-section-rtl-expected.html: Added.
  • fast/table/table-ltr-section-rtl.html: Added.
  • fast/table/table-rtl-section-ltr-expected.html: Added.
  • fast/table/table-rtl-section-ltr.html: Added.
  • fast/table/table-rtl-section-rtl-expected.html: Added.
  • fast/table/table-rtl-section-rtl.html: Added.

Bonus as there was little testing for mixed direction + separate borders.

Location:
trunk
Files:
30 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r119591 r119594  
     12012-06-06  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        Add support for direction on table row group with collapsing borders
     4        https://bugs.webkit.org/show_bug.cgi?id=87900
     5
     6        Reviewed by Ojan Vafai.
     7
     8        * fast/table/border-collapsing/first-cell-left-border-hidden-table-ltr-section-rtl-expected.html: Added.
     9        * fast/table/border-collapsing/first-cell-left-border-hidden-table-ltr-section-rtl.html: Added.
     10        * fast/table/border-collapsing/last-cell-left-border-hidden-table-ltr-section-rtl-expected.html: Added.
     11        * fast/table/border-collapsing/last-cell-left-border-hidden-table-ltr-section-rtl.html: Added.
     12        * fast/table/border-collapsing/left-border-table-ltr-section-rtl-expected.html: Added.
     13        * fast/table/border-collapsing/left-border-table-ltr-section-rtl.html: Added.
     14        * fast/table/border-collapsing/left-border-table-rtl-section-ltr-expected.html: Added.
     15        * fast/table/border-collapsing/left-border-table-rtl-section-ltr.html: Added.
     16        * fast/table/border-collapsing/left-border-table-rtl-section-rtl-expected.html: Added.
     17        * fast/table/border-collapsing/left-border-table-rtl-section-rtl.html: Added.
     18        * fast/table/border-collapsing/left-border-vertical-lr-table-ltr-section-rtl-expected.html: Added.
     19        * fast/table/border-collapsing/left-border-vertical-lr-table-ltr-section-rtl.html: Added.
     20        * fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-ltr-expected.html: Added.
     21        * fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-ltr.html: Added.
     22        * fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-rtl-expected.html: Added.
     23        * fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-rtl.html: Added.
     24        * fast/table/border-collapsing/rtl-table-left-border-hidden-expected.html: Added.
     25        * fast/table/border-collapsing/rtl-table-left-border-hidden.html: Added.
     26        * fast/table/border-collapsing/top-border-vertical-rl-table-ltr-section-rtl-expected.html: Added.
     27        * fast/table/border-collapsing/top-border-vertical-rl-table-ltr-section-rtl.html: Added.
     28        * fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-ltr-expected.html: Added.
     29        * fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-ltr.html: Added.
     30        * fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-rtl-expected.html: Added.
     31        * fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-rtl.html: Added.
     32        Most of those are test are a variation of direction on table and row group along with disabling some
     33        borders (either using 'hidden' or just disabling it by setting it to 0px).
     34
     35        * fast/table/table-ltr-section-rtl-expected.html: Added.
     36        * fast/table/table-ltr-section-rtl.html: Added.
     37        * fast/table/table-rtl-section-ltr-expected.html: Added.
     38        * fast/table/table-rtl-section-ltr.html: Added.
     39        * fast/table/table-rtl-section-rtl-expected.html: Added.
     40        * fast/table/table-rtl-section-rtl.html: Added.
     41        Bonus as there was little testing for mixed direction + separate borders.
     42
    1432012-06-06  Keyar Hood  <keyar@chromium.org>
    244

  • trunk/Source/WebCore/ChangeLog

    r119591 r119594  
     12012-06-06  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        Add support for direction on table row group with collapsing borders
     4        https://bugs.webkit.org/show_bug.cgi?id=87900
     5
     6        Reviewed by Ojan Vafai.
     7
     8        Tests: fast/table/border-collapsing/first-cell-left-border-hidden-table-ltr-section-rtl.html
     9               fast/table/border-collapsing/last-cell-left-border-hidden-table-ltr-section-rtl.html
     10               fast/table/border-collapsing/left-border-table-ltr-section-rtl.html
     11               fast/table/border-collapsing/left-border-table-rtl-section-ltr.html
     12               fast/table/border-collapsing/left-border-table-rtl-section-rtl.html
     13               fast/table/border-collapsing/left-border-vertical-lr-table-ltr-section-rtl.html
     14               fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-ltr.html
     15               fast/table/border-collapsing/left-border-vertical-lr-table-rtl-section-rtl.html
     16               fast/table/border-collapsing/rtl-table-left-border-hidden.html
     17               fast/table/border-collapsing/top-border-vertical-rl-table-ltr-section-rtl.html
     18               fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-ltr.html
     19               fast/table/border-collapsing/top-border-vertical-rl-table-rtl-section-rtl.html
     20               fast/table/table-ltr-section-rtl.html
     21               fast/table/table-rtl-section-ltr.html
     22               fast/table/table-rtl-section-rtl.html
     23
     24        This change enables proper support for direction on table row group.
     25        The current code would allow people to set direction on the row group but would still
     26        use the table's code for directionality checks (with surprising consequences).
     27
     28        The main change involve swapping end / start borders in case of mixed directionality
     29        as those 2 are not in sync.
     30       
     31        Example: <table dir="ltr"><tbody dir="rtl"><td id="cell0"></td><td id="cell1"></td></tbody></table>
     32
     33        Visually:
     34        Table: Start ---------------------> End
     35        Tbody: End <---------------------- Start
     36        Cell:  | #cell0        |           #cell1 |
     37
     38        In this example, the end table border should be compared with the tbody's start border
     39        and the last cell's (in DOM order) start border.
     40
     41        * rendering/RenderTable.cpp:
     42        (WebCore::RenderTable::tableStartBorderAdjoiningCell):
     43        (WebCore::RenderTable::tableEndBorderAdjoiningCell):
     44        Same as the other adjoining functions.
     45
     46        * rendering/RenderTable.h:
     47        (WebCore::RenderTable::lastColumnIndex):
     48        Helper function to get the last column index.
     49
     50        * rendering/RenderTableCell.cpp:
     51        (WebCore::RenderTableCell::computeCollapsedStartBorder):
     52        (WebCore::RenderTableCell::computeCollapsedEndBorder):
     53        Updated to call the table's adjoining border helpers.
     54
     55        * rendering/RenderTableCell.h:
     56        (WebCore::RenderTableCell::styleForCellFlow):
     57        Updated to use the table row group's style now that we properly support it.
     58
     59        (WebCore::RenderTableCell::isFirstOrLastCellInRow):
     60        Debug only helper to make sure we don't call the border adjoining function
     61        on non-terminal cells.
     62
     63        (WebCore::RenderTableCell::borderAdjoiningTableStart):
     64        (WebCore::RenderTableCell::borderAdjoiningTableEnd):
     65        * rendering/RenderTableRow.h:
     66        (WebCore::RenderTableRow::borderAdjoiningTableStart):
     67        (WebCore::RenderTableRow::borderAdjoiningTableEnd):
     68        * rendering/RenderTableSection.cpp:
     69        (WebCore::RenderTableSection::firstRowCellAdjoiningTableStart):
     70        (WebCore::RenderTableSection::firstRowCellAdjoiningTableEnd):
     71        Updated the previous function to account for mixed directionality.
     72
     73        (WebCore::RenderTableSection::layoutRows):
     74        Move some of the code to setLogicalPositionForCell to match RenderBlock.
     75
     76        (WebCore::RenderTableSection::setLogicalPositionForCell):
     77        Switched direction checks to styleForCellFlow (this doesn't change anything as we
     78        were already properly flipping already but ensure better).
     79
     80        * rendering/RenderTableSection.h:
     81        (WebCore::RenderTableSection::hasSameDirectionAsTable):
     82        Added this helper to know if we have a mixed direction.
     83
     84        (WebCore::RenderTableSection::borderAdjoiningTableStart):
     85        (WebCore::RenderTableSection::borderAdjoiningTableEnd):
     86        Updated to account for mixed directionality.
     87
    1882012-06-06  Keyar Hood  <keyar@chromium.org>
    289

  • trunk/Source/WebCore/rendering/RenderTable.cpp

    r119492 r119594  
    13121312}
    13131313
    1314 }
     1314const BorderValue& RenderTable::tableStartBorderAdjoiningCell(const RenderTableCell* cell) const
     1315{
     1316    ASSERT(cell->isFirstOrLastCellInRow());
     1317    if (cell->section()->hasSameDirectionAsTable())
     1318        return style()->borderStart();
     1319
     1320    return style()->borderEnd();
     1321}
     1322
     1323const BorderValue& RenderTable::tableEndBorderAdjoiningCell(const RenderTableCell* cell) const
     1324{
     1325    ASSERT(cell->isFirstOrLastCellInRow());
     1326    if (cell->section()->hasSameDirectionAsTable())
     1327        return style()->borderEnd();
     1328
     1329    return style()->borderStart();
     1330}
     1331
     1332}

  • trunk/Source/WebCore/rendering/RenderTable.h

    r119492 r119594  
    149149    RenderTableSection* topNonEmptySection() const;
    150150
     151    unsigned lastColumnIndex() const { return numEffCols() - 1; }
     152
    151153    void splitColumn(unsigned position, unsigned firstSpan);
    152154    void appendColumn(unsigned span);
    … …  
    220222        return createAnonymousWithParentRenderer(parent);
    221223    }
     224
     225    const BorderValue& tableStartBorderAdjoiningCell(const RenderTableCell*) const;
     226    const BorderValue& tableEndBorderAdjoiningCell(const RenderTableCell*) const;
    222227
    223228protected:

  • trunk/Source/WebCore/rendering/RenderTableCell.cpp

    r119507 r119594  
    469469    } else {
    470470        // (7) The table's start border.
    471         result = chooseBorder(result, CollapsedBorderValue(table->style()->borderStart(), includeColor ? table->style()->visitedDependentColor(startColorProperty) : Color(), BTABLE));
     471        result = chooseBorder(result, CollapsedBorderValue(table->tableStartBorderAdjoiningCell(this), includeColor ? table->style()->visitedDependentColor(startColorProperty) : Color(), BTABLE));
    472472        if (!result.exists())
    473473            return result;
    … …  
    543543    } else {
    544544        // (7) The table's end border.
    545         result = chooseBorder(result, CollapsedBorderValue(table->style()->borderEnd(), includeColor ? table->style()->visitedDependentColor(endColorProperty) : Color(), BTABLE));
     545        result = chooseBorder(result, CollapsedBorderValue(table->tableEndBorderAdjoiningCell(this), includeColor ? table->style()->visitedDependentColor(endColorProperty) : Color(), BTABLE));
    546546        if (!result.exists())
    547547            return result;

  • trunk/Source/WebCore/rendering/RenderTableCell.h

    r119507 r119594  
    143143    const RenderStyle* styleForCellFlow() const
    144144    {
    145         return table()->style();
     145        return section()->style();
    146146    }
    147147
    148148    const BorderValue& borderAdjoiningTableStart() const
    149149    {
     150        ASSERT(isFirstOrLastCellInRow());
     151        if (section()->hasSameDirectionAsTable())
     152            return style()->borderStart();
     153
     154        return style()->borderEnd();
     155    }
     156
     157    const BorderValue& borderAdjoiningTableEnd() const
     158    {
     159        ASSERT(isFirstOrLastCellInRow());
     160        if (section()->hasSameDirectionAsTable())
     161            return style()->borderEnd();
     162
    150163        return style()->borderStart();
    151164    }
    152165
    153     const BorderValue& borderAdjoiningTableEnd() const
    154     {
    155         return style()->borderEnd();
    156     }
    157 
     166#ifndef NDEBUG
     167    bool isFirstOrLastCellInRow() const
     168    {
     169        return !table()->cellAfter(this) || !table()->cellBefore(this);
     170    }
     171#endif
    158172protected:
    159173    virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);

  • trunk/Source/WebCore/rendering/RenderTableRow.h

    r119272 r119594  
    6969    const BorderValue& borderAdjoiningTableStart() const
    7070    {
    71         return style()->borderStart();
     71        if (section()->hasSameDirectionAsTable())
     72            return style()->borderStart();
     73
     74        return style()->borderEnd();
    7275    }
    7376
    7477    const BorderValue& borderAdjoiningTableEnd() const
    7578    {
    76         return style()->borderEnd();
     79        if (section()->hasSameDirectionAsTable())
     80            return style()->borderEnd();
     81
     82        return style()->borderStart();
    7783    }
    7884

  • trunk/Source/WebCore/rendering/RenderTableSection.cpp

    r119507 r119594  
    530530    m_forceSlowPaintPathWithOverflowingCell = false;
    531531
    532     int hspacing = table()->hBorderSpacing();
    533532    int vspacing = table()->vBorderSpacing();
    534533    unsigned nEffCols = table()->numEffCols();
    … …  
    654653            LayoutRect oldCellRect(cell->x(), cell->y() , cell->width(), cell->height());
    655654
    656             LayoutPoint cellLocation(0, m_rowPos[rindx]);
    657             // FIXME: Switch to cell's styleForCellFlow() for consistency with RenderTableCell, once it supports row group.
    658             if (!style()->isLeftToRightDirection())
    659                 cellLocation.setX(table()->columnPositions()[nEffCols] - table()->columnPositions()[table()->colToEffCol(cell->col() + cell->colSpan())] + hspacing);
    660             else
    661                 cellLocation.setX(table()->columnPositions()[c] + hspacing);
    662             cell->setLogicalLocation(cellLocation);
    663             view()->addLayoutDelta(oldCellRect.location() - cell->location());
     655            setLogicalPositionForCell(cell, c);
    664656
    665657            if (intrinsicPaddingBefore != oldIntrinsicPaddingBefore || intrinsicPaddingAfter != oldIntrinsicPaddingAfter)
    … …  
    12791271const RenderTableCell* RenderTableSection::firstRowCellAdjoiningTableStart() const
    12801272{
    1281     return cellAt(0, 0).primaryCell();
     1273    unsigned adjoiningStartCellColumnIndex = hasSameDirectionAsTable() ? 0 : table()->lastColumnIndex();
     1274    return cellAt(0, adjoiningStartCellColumnIndex).primaryCell();
    12821275}
    12831276
    12841277const RenderTableCell* RenderTableSection::firstRowCellAdjoiningTableEnd() const
    12851278{
    1286     return cellAt(0, table()->numEffCols() - 1).primaryCell();
    1287 }
    1288 
     1279    unsigned adjoiningEndCellColumnIndex = hasSameDirectionAsTable() ? table()->lastColumnIndex() : 0;
     1280    return cellAt(0, adjoiningEndCellColumnIndex).primaryCell();
     1281}
    12891282
    12901283void RenderTableSection::appendColumn(unsigned pos)
    … …  
    14311424}
    14321425
     1426void RenderTableSection::setLogicalPositionForCell(RenderTableCell* cell, unsigned effectiveColumn) const
     1427{
     1428    LayoutPoint oldCellLocation(cell->x(), cell->y());
     1429
     1430    LayoutPoint cellLocation(0, m_rowPos[cell->rowIndex()]);
     1431    int horizontalBorderSpacing = table()->hBorderSpacing();
     1432
     1433    if (!cell->styleForCellFlow()->isLeftToRightDirection())
     1434        cellLocation.setX(table()->columnPositions()[table()->numEffCols()] - table()->columnPositions()[table()->colToEffCol(cell->col() + cell->colSpan())] + horizontalBorderSpacing);
     1435    else
     1436        cellLocation.setX(table()->columnPositions()[effectiveColumn] + horizontalBorderSpacing);
     1437
     1438    cell->setLogicalLocation(cellLocation);
     1439    view()->addLayoutDelta(oldCellLocation - cell->location());
     1440}
     1441
    14331442} // namespace WebCore

  • trunk/Source/WebCore/rendering/RenderTableSection.h

    r119492 r119594  
    115115    };
    116116
     117    bool hasSameDirectionAsTable() const
     118    {
     119        return table()->style()->direction() == style()->direction();
     120    }
     121
    117122    const BorderValue& borderAdjoiningTableStart() const
    118123    {
     124        if (hasSameDirectionAsTable())
     125            return style()->borderStart();
     126
     127        return style()->borderEnd();
     128    }
     129
     130    const BorderValue& borderAdjoiningTableEnd() const
     131    {
     132        if (hasSameDirectionAsTable())
     133            return style()->borderEnd();
     134
    119135        return style()->borderStart();
    120     }
    121 
    122     const BorderValue& borderAdjoiningTableEnd() const
    123     {
    124         return style()->borderEnd();
    125136    }
    126137
    … …  
    220231    CellSpan dirtiedColumns(const LayoutRect& repaintRect) const;
    221232
     233    void setLogicalPositionForCell(RenderTableCell*, unsigned effectiveColumn) const;
     234
    222235    RenderObjectChildList m_children;
    223236
Note: See TracChangeset for help on using the changeset viewer.