Commit 7f62fff5 authored by skobes@chromium.org's avatar skobes@chromium.org

Set multiplier to 1 on blocks with no direct text children.

This fixes cases where a node that previously had a >1 multiplier ceases to have direct text children.  This can happen if its text content is removed in script, or if its 'display' style is changed from inline to block.  (There are probably other ways for it to happen also.)

Previously such a node would have a "stale" multiplier, since the autosizer only updated multipliers on text nodes and their immediate parents.  In theory the stale multiplier should not have affected the rendering, but:

- it is observable through the lineHeight property, and
- due to http://crbug.com/380903 the stale multiplier on an ancestor might be used to render text, even when the text node has the correct multiplier

BUG=378959

Review URL: https://codereview.chromium.org/319013003

git-svn-id: svn://svn.chromium.org/blink/trunk@175603 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 6e1b913d
PASS window.getComputedStyle(inlineToBlock).lineHeight is "30px"
PASS successfullyParsed is true
TEST COMPLETE
This test verifies that lineHeight values are consistent after display changes. Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisiut aliquip ex ea commodo consequat. Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisiut aliquip ex ea commodo consequat.
<!DOCTYPE html>
<meta name="viewport" content="width=800">
<script src="../../resources/js-test.js"></script>
<script src="resources/autosizingTest.js"></script>
<style>
html {
font-size: 16px;
}
body {
width: 800px;
margin: 0;
overflow-y: hidden;
}
#inlineToBlock {
display: inline;
width: 200px;
line-height: 30px;
font-size: 30px;
}
</style>
<span id="inlineToBlock">
<div>
This test verifies that lineHeight values are consistent after display changes.
Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor
incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis
nostrud exercitation ullamco laboris nisiut aliquip ex ea commodo consequat.
Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor
incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis
nostrud exercitation ullamco laboris nisiut aliquip ex ea commodo consequat.
</div>
</span>
<script>
var forceLayout1 = document.body.offsetTop;
var inlineToBlock = document.getElementById('inlineToBlock');
inlineToBlock.style.display = "block";
var forceLayout2 = document.body.offsetTop;
shouldBeEqualToString("window.getComputedStyle(inlineToBlock).lineHeight", "30px");
</script>
PASS window.getComputedStyle(span1).lineHeight is "16px"
PASS window.getComputedStyle(span2).lineHeight is "16px"
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE html>
<meta name="viewport" content="width=800">
<script src="../../resources/js-test.js"></script>
<script src="resources/autosizingTest.js"></script>
<style>
html {
font-size: 16px;
line-height: 16px;
}
body {
width: 800px;
margin: 0;
overflow-y: hidden;
}
</style>
<span id="span1">
Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor
incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis
nostrud exercitation ullamco laboris nisiut aliquip ex ea commodo consequat.
</span>
<span id="span2"></span>
<script>
var forceLayout1 = document.body.offsetTop;
var span1 = document.getElementById('span1');
var span2 = document.getElementById('span2');
span1.removeChild(span1.firstChild);
var forceLayout2 = document.body.offsetTop;
shouldBeEqualToString("window.getComputedStyle(span1).lineHeight", "16px");
shouldBeEqualToString("window.getComputedStyle(span2).lineHeight", "16px");
</script>
...@@ -372,7 +372,7 @@ void FastTextAutosizer::beginLayout(RenderBlock* block) ...@@ -372,7 +372,7 @@ void FastTextAutosizer::beginLayout(RenderBlock* block)
// Cells in auto-layout tables are handled separately by inflateAutoTable. // Cells in auto-layout tables are handled separately by inflateAutoTable.
bool isAutoTableCell = block->isTableCell() && !toRenderTableCell(block)->table()->style()->isFixedTableLayout(); bool isAutoTableCell = block->isTableCell() && !toRenderTableCell(block)->table()->style()->isFixedTableLayout();
if (block->childrenInline() && block->firstChild() && !isAutoTableCell) if (!isAutoTableCell && !m_clusterStack.isEmpty())
inflate(block); inflate(block);
} }
...@@ -441,30 +441,42 @@ void FastTextAutosizer::endLayout(RenderBlock* block) ...@@ -441,30 +441,42 @@ void FastTextAutosizer::endLayout(RenderBlock* block)
} }
} }
void FastTextAutosizer::inflate(RenderBlock* block) float FastTextAutosizer::inflate(RenderObject* parent, float multiplier)
{ {
Cluster* cluster = currentCluster(); Cluster* cluster = currentCluster();
float multiplier = 0; bool hasTextChild = false;
RenderObject* descendant = block->firstChild();
while (descendant) { RenderObject* child = 0;
// Skip block descendants because they will be inflate()'d on their own. if (parent->isRenderBlock() && parent->childrenInline())
if (descendant->isRenderBlock()) { child = toRenderBlock(parent)->firstChild();
descendant = descendant->nextInPreOrderAfterChildren(block); else if (parent->isRenderInline())
continue; child = toRenderInline(parent)->firstChild();
}
if (descendant->isText()) { while (child) {
if (child->isText()) {
hasTextChild = true;
// We only calculate this multiplier on-demand to ensure the parent block of this text // We only calculate this multiplier on-demand to ensure the parent block of this text
// has entered layout. // has entered layout.
if (!multiplier) if (!multiplier)
multiplier = cluster->m_flags & SUPPRESSING ? 1.0f : clusterMultiplier(cluster); multiplier = cluster->m_flags & SUPPRESSING ? 1.0f : clusterMultiplier(cluster);
applyMultiplier(descendant, multiplier); applyMultiplier(child, multiplier);
applyMultiplier(descendant->parent(), multiplier); // Parent handles line spacing.
// FIXME: Investigate why MarkOnlyThis is sufficient. // FIXME: Investigate why MarkOnlyThis is sufficient.
if (descendant->parent()->isRenderInline()) if (parent->isRenderInline())
descendant->setPreferredLogicalWidthsDirty(MarkOnlyThis); child->setPreferredLogicalWidthsDirty(MarkOnlyThis);
} else if (child->isRenderInline()) {
multiplier = inflate(child, multiplier);
} }
descendant = descendant->nextInPreOrder(block); child = child->nextSibling();
}
if (hasTextChild) {
applyMultiplier(parent, multiplier); // Parent handles line spacing.
} else if (!parent->isListItem()) {
// For consistency, a block with no immediate text child should always have a
// multiplier of 1 (except for list items which are handled in inflateListItem).
applyMultiplier(parent, 1);
} }
return multiplier;
} }
bool FastTextAutosizer::shouldHandleLayout() const bool FastTextAutosizer::shouldHandleLayout() const
......
...@@ -245,7 +245,7 @@ private: ...@@ -245,7 +245,7 @@ private:
void beginLayout(RenderBlock*); void beginLayout(RenderBlock*);
void endLayout(RenderBlock*); void endLayout(RenderBlock*);
void inflateAutoTable(RenderTable*); void inflateAutoTable(RenderTable*);
void inflate(RenderBlock*); float inflate(RenderObject*, float multiplier = 0);
bool shouldHandleLayout() const; bool shouldHandleLayout() const;
void setAllTextNeedsLayout(); void setAllTextNeedsLayout();
void resetMultipliers(); void resetMultipliers();
......
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