Skip to content

GitLab

T
tangled
Commit 15346c01 authored by pdr@chromium.org's avatar pdr@chromium.org
Browse files
Options

[FastTextAutosizer] Do not pre-inflate nested tables 

Instead of pre-inflating all children in inflateTable(), this patch only
pre-inflates non-table children. This fixes a bug where nested tables
had their cell widths inflated with their parent table's multiplier but
ended up not being inflated at all--leading to extra wide cells.

This fixes a bug on codereview.chromium.org where people's names would
have extra space around them (http://pr.gg/tablewidth.png).

There is the potential for regressions with this change if a table has
enough text to autosize but where the width ends up being less than
100% of the containing <td>. The real world impact script showed no
changes on the top 100 websites and hackernews (the canonical nested-
table mess) renders fine after this change.

BUG=362930

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

git-svn-id: svn://svn.chromium.org/blink/trunk@171420 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 0842b924
Hide whitespace changes
Inline Side-by-side
Showing with 69 additions and 4 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ 15346c01
......@@ -701,6 +701,8 @@ crbug.com/253763 fast/text-autosizing/first-line-scale-factor.html [ ImageOnlyFa
crbug.com/253763 fast/text-autosizing/pseudo-scale-factor.html [ ImageOnlyFailure ]
crbug.com/253763 virtual/fasttextautosizing/fast/text-autosizing/first-line-scale-factor.html [ ImageOnlyFailure ]
crbug.com/362930 virtual/fasttextautosizing/fast/text-autosizing/tables/nested-table-wrapping.html [ NeedsRebaseline ]
# FIXME(crbug.com/302005): Remove these as expectations are updated or the fasttextautosizer improves.
crbug.com/322782 virtual/fasttextautosizing/fast/text-autosizing/cluster-inline-grid-flex-box.html [ ImageOnlyFailure ]
crbug.com/322782 virtual/fasttextautosizing/fast/text-autosizing/cluster-list-item.html [ ImageOnlyFailure ]
......
third_party/WebKit/LayoutTests/virtual/fasttextautosizing/fast/text-autosizing/tables/nested-table-wrapping.html 0 → 100644
View file @ 15346c01
<!DOCTYPE html>
<html>
<head>
<meta name="viewport" content="width=800">
<style>
body {
width: 800px;
margin: 0;
overflow-y: hidden;
font-size: 12px;
}
#description {
overflow: clip;
width: 600px;
height: 3em;
}
table {
outline: 1px solid black;
margin: 1px;
border-collapse: collapse;
}
td {
background-color: honeydew;
border: 1px dotted darkgreen;
}
</style>
<script>
if (window.internals) {
window.internals.settings.setTextAutosizingEnabled(true);
window.internals.settings.setTextAutosizingWindowSizeOverride(320, 480);
} else if (window.console && console.warn) {
console.warn("This test depends on the Text Autosizing setting being true, so run it in DumpRenderTree, or manually enable Text Autosizing, and either use a mobile device with 320px device-width (like Nexus S or iPhone), or define HACK_FORCE_TEXT_AUTOSIZING_ON_DESKTOP.");
}
</script>
</head>
<body>
<div id="description">
Table autosizing tests - nested-table-wrapping.html<br/>
This test passes if "the table cell should tightly wrap this text" is tightly wrapped.
</div>
<table>
<tr>
<td>
Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize Autosize
<table>
<tr>
<td>the table cell should tightly wrap this text</td>
</tr>
</table>
</td>
</tr>
</table>
</body>
</html>
third_party/WebKit/Source/core/rendering/FastTextAutosizer.cpp
View file @ 15346c01
......@@ -402,10 +402,17 @@ void FastTextAutosizer::inflateTable(RenderTable* table)
shouldAutosize = clusterWouldHaveEnoughTextToAutosize(renderTableCell, table);
if (shouldAutosize) {
for (RenderObject* child = cell; child; child = child->nextInPreOrder(cell)) {
if (child->isText() && child->needsLayout()) {
applyMultiplier(child, multiplier);
applyMultiplier(child->parent(), multiplier); // Parent handles line spacing.
RenderObject* child = cell;
while (child) {
if (child->needsLayout() && !child->isTable()) {
if (child->isText()) {
applyMultiplier(child, multiplier);
applyMultiplier(child->parent(), multiplier); // Parent handles line spacing.
}
child = child->nextInPreOrder(cell);
} else {
// Skip inflation of this subtree.
child = child->nextInPreOrderAfterChildren(cell);
}
}
}
......
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