Commit 9c820804 authored by wangxianzhu's avatar wangxianzhu Committed by Commit bot

Fix LayoutBox::topLeftLocation for table rows and table cells

Previously we used containingBlock() as the default flipped blocks
container in LayoutBox::topLeftLocation(). This was incorrect for
table rows and table cells whose locations and topLeftLocations
should be relative to the containing section.

This fixes layout tests about table row/cells in vertical writing
mode for slimmingPaintInvalidation.

BUG=652392

Review-Url: https://codereview.chromium.org/2392503003
Cr-Commit-Position: refs/heads/master@{#422917}
parent d0717890
......@@ -2283,11 +2283,10 @@ bool LayoutBox::mapToVisualRectInAncestorSpace(
LayoutObject* container =
this->container(ancestor, &ancestorSkipped, &filterSkipped);
LayoutBox* tableRowContainer = nullptr;
// Skip table row because cells and rows are in the same coordinate space
// (see below, however for more comments about when |ancestor| is the table row).
// The second and third conditionals below are to skip cases where content has display: table-row or display: table-cell but is not
// parented like a cell/row combo.
if (container->isTableRow() && isTableCell() && parentBox() == container) {
// Skip table row because cells and rows are in the same coordinate space (see
// below, however for more comments about when |ancestor| is the table row).
if (isTableCell()) {
DCHECK(container->isTableRow() && parentBox() == container);
if (container != ancestor)
container = container->parent();
else
......@@ -5048,11 +5047,24 @@ LayoutPoint LayoutBox::flipForWritingModeForChild(
point.y());
}
LayoutBox* LayoutBox::locationContainer() const {
// Normally the box's location is relative to its containing box.
LayoutObject* container = this->container();
while (container && !container->isBox())
container = container->container();
return toLayoutBox(container);
}
LayoutPoint LayoutBox::topLeftLocation(
const LayoutBox* flippedBlocksContainer) const {
const LayoutBox* containerBox =
flippedBlocksContainer ? flippedBlocksContainer : containingBlock();
if (!containerBox || containerBox == this)
const LayoutBox* containerBox;
if (flippedBlocksContainer) {
DCHECK(flippedBlocksContainer == locationContainer());
containerBox = flippedBlocksContainer;
} else {
containerBox = locationContainer();
}
if (!containerBox)
return location();
return containerBox->flipForWritingModeForChild(this, location());
}
......
......@@ -326,6 +326,10 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject {
frameRectChanged();
}
// The ancestor box that this object's location and topLeftLocation are
// relative to.
virtual LayoutBox* locationContainer() const;
// FIXME: Currently scrollbars are using int geometry and positioned based on
// pixelSnappedBorderBoxRect whose size may change when location changes because of
// pixel snapping. This function is used to change location of the LayoutBox outside
......
......@@ -100,4 +100,89 @@ TEST_F(LayoutBoxTest, BackgroundRect) {
layoutBox->backgroundRect(BackgroundClipRect));
}
TEST_F(LayoutBoxTest, LocationContainer) {
setBodyInnerHTML(
"<div id='div'>"
" <b>Inline content<img id='img'></b>"
"</div>"
"<table id='table'>"
" <tbody id='tbody'>"
" <tr id='row'>"
" <td id='cell' style='width: 100px; height: 80px'></td>"
" </tr>"
" </tbody>"
"</table>");
const LayoutBox* body = document().body()->layoutBox();
const LayoutBox* div = toLayoutBox(getLayoutObjectByElementId("div"));
const LayoutBox* img = toLayoutBox(getLayoutObjectByElementId("img"));
const LayoutBox* table = toLayoutBox(getLayoutObjectByElementId("table"));
const LayoutBox* tbody = toLayoutBox(getLayoutObjectByElementId("tbody"));
const LayoutBox* row = toLayoutBox(getLayoutObjectByElementId("row"));
const LayoutBox* cell = toLayoutBox(getLayoutObjectByElementId("cell"));
EXPECT_EQ(body, div->locationContainer());
EXPECT_EQ(div, img->locationContainer());
EXPECT_EQ(body, table->locationContainer());
EXPECT_EQ(table, tbody->locationContainer());
EXPECT_EQ(tbody, row->locationContainer());
EXPECT_EQ(tbody, cell->locationContainer());
}
TEST_F(LayoutBoxTest, TopLeftLocationFlipped) {
setBodyInnerHTML(
"<div style='width: 600px; height: 200px; writing-mode: vertical-rl'>"
" <div id='box1' style='width: 100px'></div>"
" <div id='box2' style='width: 200px'></div>"
"</div>");
const LayoutBox* box1 = toLayoutBox(getLayoutObjectByElementId("box1"));
EXPECT_EQ(LayoutPoint(0, 0), box1->location());
EXPECT_EQ(LayoutPoint(500, 0), box1->topLeftLocation());
const LayoutBox* box2 = toLayoutBox(getLayoutObjectByElementId("box2"));
EXPECT_EQ(LayoutPoint(100, 0), box2->location());
EXPECT_EQ(LayoutPoint(300, 0), box2->topLeftLocation());
}
TEST_F(LayoutBoxTest, TableRowCellTopLeftLocationFlipped) {
setBodyInnerHTML(
"<div style='writing-mode: vertical-rl'>"
" <table style='border-spacing: 0'>"
" <thead><tr><td style='width: 50px'></td></tr></thead>"
" <tbody>"
" <tr id='row1'>"
" <td id='cell1' style='width: 100px; height: 80px'></td>"
" </tr>"
" <tr id='row2'>"
" <td id='cell2' style='width: 300px; height: 80px'></td>"
" </tr>"
" </tbody>"
" </table>"
"</div>");
// location and topLeftLocation of a table row or a table cell should be
// relative to the containing section.
const LayoutBox* row1 = toLayoutBox(getLayoutObjectByElementId("row1"));
EXPECT_EQ(LayoutPoint(0, 0), row1->location());
EXPECT_EQ(LayoutPoint(300, 0), row1->topLeftLocation());
const LayoutBox* cell1 = toLayoutBox(getLayoutObjectByElementId("cell1"));
EXPECT_EQ(LayoutPoint(0, 0), cell1->location());
EXPECT_EQ(LayoutPoint(300, 0), cell1->topLeftLocation());
const LayoutBox* row2 = toLayoutBox(getLayoutObjectByElementId("row2"));
// TODO(crbug.com/652496): LayoutTableRow's location is in logical coordinates
// of the containing section, and topLeftLocation() is incorrect.
// This should be (100, 0).
EXPECT_EQ(LayoutPoint(0, 100), row2->location());
// This should be (0, 0).
EXPECT_EQ(LayoutPoint(100, 100), row2->topLeftLocation());
const LayoutBox* cell2 = toLayoutBox(getLayoutObjectByElementId("cell2"));
EXPECT_EQ(LayoutPoint(100, 0), cell2->location());
EXPECT_EQ(LayoutPoint(0, 0), cell2->topLeftLocation());
}
} // namespace blink
......@@ -288,6 +288,9 @@ class CORE_EXPORT LayoutTableCell final : public LayoutBlockFlow {
void adjustChildDebugRect(LayoutRect&) const override;
// A table cell's location is relative to its containing section.
LayoutBox* locationContainer() const override { return section(); }
protected:
void styleDidChange(StyleDifference, const ComputedStyle* oldStyle) override;
void computePreferredLogicalWidths() override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment