Changeset 131671 in webkit


Ignore:
Timestamp:
Oct 17, 2012 5:00:55 PM (12 years ago)
Author:
commit-queue@webkit.org
Message:

Incorrect rendering of borders on <col> with span > 1
​https://bugs.webkit.org/show_bug.cgi?id=76246

Patch by Arpita Bahuguna <​arpitabahuguna@gmail.com> on 2012-10-17
Reviewed by Julien Chaffraix.

Source/WebCore:

The HTML5 rendering specification [10.2.2 - Display Types] states that
"For the purposes of the CSS table model, the col element is expected to
be treated as if it was present as many times as its span attribute
specifies."
We should thus apply a col element's border as if the element is present
as many number of times as its span attribute.

Apart from this, we should also treat the col and its enclosing colgroup
separately while computing the collapsed borders.

Test: fast/table/border-collapsing/collapsed-border-with-col-colgroup-span.html

  • rendering/RenderTableCell.cpp:

(WebCore::RenderTableCell::computeCollapsedStartBorder):
(WebCore::RenderTableCell::computeCollapsedEndBorder):
Borders from col and its enclosing colgroup element should be handled
separately, while considering the preceeding col's end border (for start
border computation) and the next col's start border (for end border
computation).

Also, have made changes for handling of col elements with span attribute as
per the specification. We now apply the border (start or end) of the col
element irrespective of whether it has any span specified for it or not.

LayoutTests:

  • fast/table/border-collapsing/cached-change-colgroup-border-color-expected.png:
  • fast/table/border-collapsing/cached-change-colgroup-border-color-expected.txt:
  • fast/table/border-collapsing/cached-change-colgroup-border-width-expected.png:
  • fast/table/border-collapsing/cached-change-colgroup-border-width-expected.txt:

Existing tests modified. This is because previously, while computing collapsed
start border, we did not take the preceeding col's enclosing colgroup's end border
into consideration. While computing the collapsed start border, only the preceeding
col element's end border was considered.

With this fix, for the above two tests, the last col's width now changes due to
the border being applied to it (the preceeding col's enclosing colgroup's end border)
which causes the table's width to change. Also, we should note that the cell's grow
by half the border's width; which is expected (on account of their being collapsed
borders).

  • fast/table/border-collapsing/collapsed-border-with-col-colgroup-span-expected.txt: Added.
  • fast/table/border-collapsing/collapsed-border-with-col-colgroup-span.html: Added.
  • platform/chromium-linux/fast/table/border-collapsing/collapsed-border-with-col-colgroup-span-expected.png: Added.

Image only test added for verifying the behavior of collapsed borders with
col and colgroup span.
New expected image file added for the chromium-linux port.

Location:
trunk
Files:
2 added
7 edited
1 copied

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r131670 r131671  
     12012-10-17  Arpita Bahuguna  <arpitabahuguna@gmail.com>
     2
     3        Incorrect rendering of borders on <col> with span > 1
     4        https://bugs.webkit.org/show_bug.cgi?id=76246
     5
     6        Reviewed by Julien Chaffraix.
     7
     8        * fast/table/border-collapsing/cached-change-colgroup-border-color-expected.png:
     9        * fast/table/border-collapsing/cached-change-colgroup-border-color-expected.txt:
     10        * fast/table/border-collapsing/cached-change-colgroup-border-width-expected.png:
     11        * fast/table/border-collapsing/cached-change-colgroup-border-width-expected.txt:
     12        Existing tests modified. This is because previously, while computing collapsed
     13        start border, we did not take the preceeding col's enclosing colgroup's end border
     14        into consideration. While computing the collapsed start border, only the preceeding
     15        col element's end border was considered.
     16
     17        With this fix, for the above two tests, the last col's width now changes due to
     18        the border being applied to it (the preceeding col's enclosing colgroup's end border)
     19        which causes the table's width to change. Also, we should note that the cell's grow
     20        by half the border's width; which is expected (on account of their being collapsed
     21        borders).
     22
     23        * fast/table/border-collapsing/collapsed-border-with-col-colgroup-span-expected.txt: Added.
     24        * fast/table/border-collapsing/collapsed-border-with-col-colgroup-span.html: Added.
     25        * platform/chromium-linux/fast/table/border-collapsing/collapsed-border-with-col-colgroup-span-expected.png: Added.
     26        Image only test added for verifying the behavior of collapsed borders with
     27        col and colgroup span.
     28        New expected image file added for the chromium-linux port.
     29
    1302012-10-17  Tom Sepez  <tsepez@chromium.org>
    231

  • trunk/LayoutTests/fast/table/border-collapsing/cached-change-colgroup-border-color-expected.txt

    r95852 r131671  
    44  RenderBlock {HTML} at (0,0) size 800x600
    55    RenderBody {BODY} at (8,8) size 784x584
    6       RenderTable {TABLE} at (0,0) size 164x102 [border: (1px solid #0000FF)]
     6      RenderTable {TABLE} at (0,0) size 166x102 [border: (1px solid #0000FF)]
    77        RenderTableCol {COLGROUP} at (0,0) size 0x0 [border: (4px solid #FFFF00)]
    88          RenderTableCol {COL} at (0,0) size 0x0
    … …  
    1010        RenderTableCol {COLGROUP} at (0,0) size 0x0
    1111          RenderTableCol {COL} at (0,0) size 0x0
    12         RenderTableSection {TBODY} at (1,1) size 162x100
    13           RenderTableRow {TR} at (0,0) size 162x50
     12        RenderTableSection {TBODY} at (1,1) size 164x100
     13          RenderTableRow {TR} at (0,0) size 164x50
    1414            RenderTableCell {TD} at (0,22) size 55x5 [border: (2px solid #00FF00)] [r=0 c=0 rs=1 cs=1]
    1515            RenderTableCell {TD} at (55,23) size 55x4 [border: (2px none #000000)] [r=0 c=1 rs=1 cs=1]
    16             RenderTableCell {TD} at (110,23) size 52x3 [border: (1px none #000000)] [r=0 c=2 rs=1 cs=1]
    17           RenderTableRow {TR} at (0,50) size 162x50
     16            RenderTableCell {TD} at (110,23) size 54x3 [border: (1px none #000000)] [r=0 c=2 rs=1 cs=1]
     17          RenderTableRow {TR} at (0,50) size 164x50
    1818            RenderTableCell {TD} at (0,72) size 55x5 [border: (1px none #000000)] [r=1 c=0 rs=1 cs=1]
    1919            RenderTableCell {TD} at (55,73) size 55x4 [border: none] [r=1 c=1 rs=1 cs=1]
    20             RenderTableCell {TD} at (110,74) size 52x2 [r=1 c=2 rs=1 cs=1]
     20            RenderTableCell {TD} at (110,74) size 54x2 [border: none] [r=1 c=2 rs=1 cs=1]

  • trunk/LayoutTests/fast/table/border-collapsing/cached-change-colgroup-border-width-expected.txt

    r95852 r131671  
    44  RenderBlock {HTML} at (0,0) size 800x600
    55    RenderBody {BODY} at (8,8) size 784x584
    6       RenderTable {TABLE} at (0,0) size 161x102 [border: (1px solid #0000FF)]
     6      RenderTable {TABLE} at (0,0) size 162x102 [border: (1px solid #0000FF)]
    77        RenderTableCol {COLGROUP} at (0,0) size 0x0 [border: (4px solid #FFFF00)]
    88          RenderTableCol {COL} at (0,0) size 0x0
    … …  
    1010        RenderTableCol {COLGROUP} at (0,0) size 0x0
    1111          RenderTableCol {COL} at (0,0) size 0x0
    12         RenderTableSection {TBODY} at (1,1) size 159x100
    13           RenderTableRow {TR} at (0,0) size 159x50
     12        RenderTableSection {TBODY} at (1,1) size 160x100
     13          RenderTableRow {TR} at (0,0) size 160x50
    1414            RenderTableCell {TD} at (0,22) size 53x5 [border: (2px solid #00FF00)] [r=0 c=0 rs=1 cs=1]
    1515            RenderTableCell {TD} at (53,23) size 54x4 [border: (2px none #000000)] [r=0 c=1 rs=1 cs=1]
    16             RenderTableCell {TD} at (107,23) size 52x3 [border: (1px none #000000)] [r=0 c=2 rs=1 cs=1]
    17           RenderTableRow {TR} at (0,50) size 159x50
     16            RenderTableCell {TD} at (107,23) size 53x3 [border: (1px none #000000)] [r=0 c=2 rs=1 cs=1]
     17          RenderTableRow {TR} at (0,50) size 160x50
    1818            RenderTableCell {TD} at (0,72) size 53x5 [border: (1px none #000000)] [r=1 c=0 rs=1 cs=1]
    1919            RenderTableCell {TD} at (53,73) size 54x4 [border: none] [r=1 c=1 rs=1 cs=1]
    20             RenderTableCell {TD} at (107,74) size 52x2 [r=1 c=2 rs=1 cs=1]
     20            RenderTableCell {TD} at (107,74) size 53x2 [border: none] [r=1 c=2 rs=1 cs=1]

  • trunk/Source/WebCore/ChangeLog

    r131670 r131671  
     12012-10-17  Arpita Bahuguna  <arpitabahuguna@gmail.com>
     2
     3        Incorrect rendering of borders on <col> with span > 1
     4        https://bugs.webkit.org/show_bug.cgi?id=76246
     5
     6        Reviewed by Julien Chaffraix.
     7
     8        The HTML5 rendering specification [10.2.2 - Display Types] states that
     9        "For the purposes of the CSS table model, the col element is expected to
     10        be treated as if it was present as many times as its span attribute
     11        specifies."
     12        We should thus apply a col element's border as if the element is present
     13        as many number of times as its span attribute.
     14
     15        Apart from this, we should also treat the col and its enclosing colgroup
     16        separately while computing the collapsed borders.
     17
     18        Test: fast/table/border-collapsing/collapsed-border-with-col-colgroup-span.html
     19
     20        * rendering/RenderTableCell.cpp:
     21        (WebCore::RenderTableCell::computeCollapsedStartBorder):
     22        (WebCore::RenderTableCell::computeCollapsedEndBorder):
     23        Borders from col and its enclosing colgroup element should be handled
     24        separately, while considering the preceeding col's end border (for start
     25        border computation) and the next col's start border (for end border
     26        computation).
     27
     28        Also, have made changes for handling of col elements with span attribute as
     29        per the specification. We now apply the border (start or end) of the col
     30        element irrespective of whether it has any span specified for it or not.
     31
    1322012-10-17  Tom Sepez  <tsepez@chromium.org>
    233

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

    r131465 r131671  
    501501    bool startColEdge;
    502502    bool endColEdge;
    503     RenderTableCol* colElt = table->colElement(col(), &startColEdge, &endColEdge);
    504     if (colElt && startColEdge) {
    505         result = chooseBorder(result, CollapsedBorderValue(colElt->borderAdjoiningCellStartBorder(this), includeColor ? colElt->style()->visitedDependentColor(startColorProperty) : Color(), BCOL));
    506         if (!result.exists())
    507             return result;
    508         if (RenderTableCol* enclosingColumnGroup = colElt->enclosingColumnGroupIfAdjacentBefore()) {
    509             result = chooseBorder(result, CollapsedBorderValue(enclosingColumnGroup->borderAdjoiningCellStartBorder(this), includeColor ? enclosingColumnGroup->style()->visitedDependentColor(startColorProperty) : Color(), BCOLGROUP));
     503    if (RenderTableCol* colElt = table->colElement(col(), &startColEdge, &endColEdge)) {
     504        if (colElt->isTableColumnGroup() && startColEdge) {
     505            // The |colElt| is a column group and is also the first colgroup (in case of spanned colgroups).
     506            result = chooseBorder(result, CollapsedBorderValue(colElt->borderAdjoiningCellStartBorder(this), includeColor ? colElt->style()->visitedDependentColor(startColorProperty) : Color(), BCOLGROUP));
    510507            if (!result.exists())
    511508                return result;
     509        } else if (!colElt->isTableColumnGroup()) {
     510            // We first consider the |colElt| and irrespective of whether it is a spanned col or not, we apply
     511            // its start border. This is as per HTML5 which states that: "For the purposes of the CSS table model,
     512            // the col element is expected to be treated as if it was present as many times as its span attribute specifies".
     513            result = chooseBorder(result, CollapsedBorderValue(colElt->borderAdjoiningCellStartBorder(this), includeColor ? colElt->style()->visitedDependentColor(startColorProperty) : Color(), BCOL));
     514            if (!result.exists())
     515                return result;
     516            // Next, apply the start border of the enclosing colgroup but only if it is adjacent to the cell's edge.
     517            if (RenderTableCol* enclosingColumnGroup = colElt->enclosingColumnGroupIfAdjacentBefore()) {
     518                result = chooseBorder(result, CollapsedBorderValue(enclosingColumnGroup->borderAdjoiningCellStartBorder(this), includeColor ? enclosingColumnGroup->style()->visitedDependentColor(startColorProperty) : Color(), BCOLGROUP));
     519                if (!result.exists())
     520                    return result;
     521            }
    512522        }
    513523    }
    … …  
    515525    // (6) The end border of the preceding column.
    516526    if (cellBefore) {
    517         colElt = table->colElement(col() -1, &startColEdge, &endColEdge);
    518         if (colElt && endColEdge) {
    519             CollapsedBorderValue columnBeforeAdjoiningBorder = CollapsedBorderValue(colElt->borderAdjoiningCellAfter(this), includeColor ? colElt->style()->visitedDependentColor(endColorProperty) : Color(), BCOL);
    520             result = chooseBorder(columnBeforeAdjoiningBorder, result);
    521             if (!result.exists())
    522                 return result;
     527        if (RenderTableCol* colElt = table->colElement(col() - 1, &startColEdge, &endColEdge)) {
     528            if (colElt->isTableColumnGroup() && endColEdge) {
     529                // The element is a colgroup and is also the last colgroup (in case of spanned colgroups).
     530                result = chooseBorder(CollapsedBorderValue(colElt->borderAdjoiningCellAfter(this), includeColor ? colElt->style()->visitedDependentColor(endColorProperty) : Color(), BCOLGROUP), result);
     531                if (!result.exists())
     532                    return result;
     533            } else if (colElt->isTableColumn()) {
     534                // Resolve the collapsing border against the col's border ignoring any 'span' as per HTML5.
     535                result = chooseBorder(CollapsedBorderValue(colElt->borderAdjoiningCellAfter(this), includeColor ? colElt->style()->visitedDependentColor(endColorProperty) : Color(), BCOL), result);
     536                if (!result.exists())
     537                    return result;
     538                // Next, if the previous col has a parent colgroup then its end border should be applied
     539                // but only if it is adjacent to the cell's edge.
     540                if (RenderTableCol* enclosingColumnGroup = colElt->enclosingColumnGroupIfAdjacentAfter()) {
     541                    result = chooseBorder(CollapsedBorderValue(enclosingColumnGroup->borderAdjoiningCellEndBorder(this), includeColor ? enclosingColumnGroup->style()->visitedDependentColor(endColorProperty) : Color(), BCOLGROUP), result);
     542                    if (!result.exists())
     543                        return result;
     544                }
     545            }
    523546        }
    524547    } else {
    … …  
    576599    bool startColEdge;
    577600    bool endColEdge;
    578     RenderTableCol* colElt = table->colElement(col() + colSpan() - 1, &startColEdge, &endColEdge);
    579     if (colElt && endColEdge) {
    580         result = chooseBorder(result, CollapsedBorderValue(colElt->borderAdjoiningCellEndBorder(this), includeColor ? colElt->style()->visitedDependentColor(endColorProperty) : Color(), BCOL));
    581         if (!result.exists())
    582             return result;
    583         if (RenderTableCol* enclosingColumnGroup = colElt->enclosingColumnGroupIfAdjacentAfter()) {
    584             result = chooseBorder(result, CollapsedBorderValue(enclosingColumnGroup->borderAdjoiningCellEndBorder(this), includeColor ? enclosingColumnGroup->style()->visitedDependentColor(endColorProperty) : Color(), BCOLGROUP));
     601    if (RenderTableCol* colElt = table->colElement(col() + colSpan() - 1, &startColEdge, &endColEdge)) {
     602        if (colElt->isTableColumnGroup() && endColEdge) {
     603            // The element is a colgroup and is also the last colgroup (in case of spanned colgroups).
     604            result = chooseBorder(result, CollapsedBorderValue(colElt->borderAdjoiningCellEndBorder(this), includeColor ? colElt->style()->visitedDependentColor(endColorProperty) : Color(), BCOLGROUP));
    585605            if (!result.exists())
    586606                return result;
     607        } else if (!colElt->isTableColumnGroup()) {
     608            // First apply the end border of the column irrespective of whether it is spanned or not. This is as per
     609            // HTML5 which states that: "For the purposes of the CSS table model, the col element is expected to be
     610            // treated as if it was present as many times as its span attribute specifies".
     611            result = chooseBorder(result, CollapsedBorderValue(colElt->borderAdjoiningCellEndBorder(this), includeColor ? colElt->style()->visitedDependentColor(endColorProperty) : Color(), BCOL));
     612            if (!result.exists())
     613                return result;
     614            // Next, if it has a parent colgroup then we apply its end border but only if it is adjacent to the cell.
     615            if (RenderTableCol* enclosingColumnGroup = colElt->enclosingColumnGroupIfAdjacentAfter()) {
     616                result = chooseBorder(result, CollapsedBorderValue(enclosingColumnGroup->borderAdjoiningCellEndBorder(this), includeColor ? enclosingColumnGroup->style()->visitedDependentColor(endColorProperty) : Color(), BCOLGROUP));
     617                if (!result.exists())
     618                    return result;
     619            }
    587620        }
    588621    }
    … …  
    590623    // (6) The start border of the next column.
    591624    if (!isEndColumn) {
    592         colElt = table->colElement(col() + colSpan(), &startColEdge, &endColEdge);
    593         if (colElt && startColEdge) {
    594             CollapsedBorderValue columnAfterAdjoiningBorder = CollapsedBorderValue(colElt->borderAdjoiningCellBefore(this), includeColor ? colElt->style()->visitedDependentColor(startColorProperty) : Color(), BCOL);
    595             result = chooseBorder(result, columnAfterAdjoiningBorder);
    596             if (!result.exists())
    597                 return result;
     625        if (RenderTableCol* colElt = table->colElement(col() + colSpan(), &startColEdge, &endColEdge)) {
     626            if (colElt->isTableColumnGroup() && startColEdge) {
     627                // This case is a colgroup without any col, we only compute it if it is adjacent to the cell's edge.
     628                result = chooseBorder(result, CollapsedBorderValue(colElt->borderAdjoiningCellBefore(this), includeColor ? colElt->style()->visitedDependentColor(startColorProperty) : Color(), BCOLGROUP));
     629                if (!result.exists())
     630                    return result;
     631            } else if (colElt->isTableColumn()) {
     632                // Resolve the collapsing border against the col's border ignoring any 'span' as per HTML5.
     633                result = chooseBorder(result, CollapsedBorderValue(colElt->borderAdjoiningCellBefore(this), includeColor ? colElt->style()->visitedDependentColor(startColorProperty) : Color(), BCOL));
     634                if (!result.exists())
     635                    return result;
     636                // If we have a parent colgroup, resolve the border only if it is adjacent to the cell.
     637                if (RenderTableCol* enclosingColumnGroup = colElt->enclosingColumnGroupIfAdjacentBefore()) {
     638                    result = chooseBorder(result, CollapsedBorderValue(enclosingColumnGroup->borderAdjoiningCellStartBorder(this), includeColor ? enclosingColumnGroup->style()->visitedDependentColor(startColorProperty) : Color(), BCOLGROUP));
     639                    if (!result.exists())
     640                        return result;
     641                }
     642            }
    598643        }
    599644    } else {
Note: See TracChangeset for help on using the changeset viewer.