Text Autosizing: Don't autosize unwrappable blocks
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2012 11:58:39 +0000 (11:58 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2012 11:58:39 +0000 (11:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=104925

Patch by John Mellor <johnme@chromium.org> on 2012-12-14
Reviewed by Julien Chaffraix.

Source/WebCore:

If we autosize an unwrappable block (white-space:nowrap/pre), it'll
expand sideways. This doesn't actually improve its legibility, and it
can often severely break web page layouts. This patch prevents us from
autosizing unwrappable blocks. A follow-up patch will address the more
complex issue of unwrappable inline elements.

Tests: fast/text-autosizing/unwrappable-blocks.html
       fast/text-autosizing/unwrappable-inlines.html

* rendering/TextAutosizer.cpp:
(WebCore::TextAutosizer::processContainer):
    Use containerShouldbeAutosized instead of contentHeightIsConstrained.
(WebCore::contentHeightIsConstrained):
    Unchanged, just moved lower down the file.
(WebCore::TextAutosizer::containerShouldbeAutosized):
    Checks that the block is wrappable, and also that contentHeightIsConstrained is false.
(WebCore::TextAutosizer::measureDescendantTextWidth):
    Use containerShouldbeAutosized instead of contentHeightIsConstrained.
* rendering/TextAutosizer.h:
    Declared containerShouldbeAutosized.

LayoutTests:

Added tests verifying that this prevents unwrappable blocks from being
autosized, and that this has no effect on unwrappable inlines.

* fast/text-autosizing/unwrappable-blocks-expected.html: Added.
* fast/text-autosizing/unwrappable-blocks.html: Added.
* fast/text-autosizing/unwrappable-inlines-expected.html: Added.
* fast/text-autosizing/unwrappable-inlines.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@137738 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/text-autosizing/unwrappable-blocks-expected.html [new file with mode: 0644]
LayoutTests/fast/text-autosizing/unwrappable-blocks.html [new file with mode: 0644]
LayoutTests/fast/text-autosizing/unwrappable-inlines-expected.html [new file with mode: 0644]
LayoutTests/fast/text-autosizing/unwrappable-inlines.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/TextAutosizer.cpp
Source/WebCore/rendering/TextAutosizer.h

index f567865..c9c42fe 100644 (file)
@@ -1,3 +1,18 @@
+2012-12-14  John Mellor  <johnme@chromium.org>
+
+        Text Autosizing: Don't autosize unwrappable blocks
+        https://bugs.webkit.org/show_bug.cgi?id=104925
+
+        Reviewed by Julien Chaffraix.
+
+        Added tests verifying that this prevents unwrappable blocks from being
+        autosized, and that this has no effect on unwrappable inlines.
+
+        * fast/text-autosizing/unwrappable-blocks-expected.html: Added.
+        * fast/text-autosizing/unwrappable-blocks.html: Added.
+        * fast/text-autosizing/unwrappable-inlines-expected.html: Added.
+        * fast/text-autosizing/unwrappable-inlines.html: Added.
+
 2012-12-14  Kentaro Hara  <haraken@chromium.org>
 
         Remove an exception message from insertedIntoDocument-no-crash-expected.txt
diff --git a/LayoutTests/fast/text-autosizing/unwrappable-blocks-expected.html b/LayoutTests/fast/text-autosizing/unwrappable-blocks-expected.html
new file mode 100644 (file)
index 0000000..48d3b88
--- /dev/null
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+
+<meta name="viewport" content="width=800">
+<style>
+html { font-size: 16px; }
+body { width: 800px; margin: 0; }
+pre { margin: 0; }
+</style>
+
+</head>
+<body>
+
+<pre>
+    This text should not be autosized, as &lt;pre&gt; elements have white-space:pre which cannot be wrapped.<br>
+    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 nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
+</pre>
+
+<div style="white-space: nowrap">
+    This text should not be autosized, as it has white-space:nowrap which cannot be wrapped.<br>
+    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 nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
+</div>
+
+<pre style="white-space: pre-wrap; font-size: 2.5rem">
+    This text should be autosized to 40px computed font-size (16 * 800/320) since it has white-space:pre-wrap which allows text wrapping (even though it doesn't collapse white space).
+</pre>
+
+<div style="font-size: 2.5rem">
+    This text should be autosized to 40px computed font-size (16 * 800/320).
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/text-autosizing/unwrappable-blocks.html b/LayoutTests/fast/text-autosizing/unwrappable-blocks.html
new file mode 100644 (file)
index 0000000..b6603ae
--- /dev/null
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+
+<meta name="viewport" content="width=800">
+<style>
+html { font-size: 16px; }
+body { width: 800px; margin: 0; }
+pre { margin: 0; }
+</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>
+
+<pre>
+    This text should not be autosized, as &lt;pre&gt; elements have white-space:pre which cannot be wrapped.<br>
+    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 nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
+</pre>
+
+<div style="white-space: nowrap">
+    This text should not be autosized, as it has white-space:nowrap which cannot be wrapped.<br>
+    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 nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
+</div>
+
+<pre style="white-space: pre-wrap">
+    This text should be autosized to 40px computed font-size (16 * 800/320) since it has white-space:pre-wrap which allows text wrapping (even though it doesn't collapse white space).
+</pre>
+
+<div>
+    This text should be autosized to 40px computed font-size (16 * 800/320).
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/text-autosizing/unwrappable-inlines-expected.html b/LayoutTests/fast/text-autosizing/unwrappable-inlines-expected.html
new file mode 100644 (file)
index 0000000..48fc903
--- /dev/null
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+<head>
+
+<meta name="viewport" content="width=800">
+<style>
+html { font-size: 16px; }
+body { width: 800px; margin: 0; }
+pre { margin: 0; }
+</style>
+
+</head>
+<body>
+
+<div>
+<span style="white-space: pre; font-size: 2.5rem">
+    This text is expected be autosized to 40px computed font-size (16 * 800/320), since Text Autosizing is not yet smart enough to detect that all the children of this div are unwrappable. Ideally it would not get autosized.
+</span>
+</div>
+
+<div>
+<span style="white-space: nowrap; font-size: 2.5rem">
+    This text is expected be autosized to 40px computed font-size (16 * 800/320), since Text Autosizing is not yet smart enough to detect that all the children of this div are unwrappable. Ideally it would not get autosized.
+</span>
+</div>
+
+<div>
+<span style="white-space: pre-wrap; font-size: 2.5rem">
+    This text should be autosized to 40px computed font-size (16 * 800/320) since it has white-space:pre-wrap which allows text wrapping (even though it doesn't collapse white space).
+</span>
+</div>
+
+<div>
+<span style="font-size: 2.5rem">
+    This text should be autosized to 40px computed font-size (16 * 800/320).
+</span>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/text-autosizing/unwrappable-inlines.html b/LayoutTests/fast/text-autosizing/unwrappable-inlines.html
new file mode 100644 (file)
index 0000000..9b0732e
--- /dev/null
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<head>
+
+<meta name="viewport" content="width=800">
+<style>
+html { font-size: 16px; }
+body { width: 800px; margin: 0; }
+pre { margin: 0; }
+</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>
+<span style="white-space: pre">
+    This text is expected be autosized to 40px computed font-size (16 * 800/320), since Text Autosizing is not yet smart enough to detect that all the children of this div are unwrappable. Ideally it would not get autosized.
+</span>
+</div>
+
+<div>
+<span style="white-space: nowrap">
+    This text is expected be autosized to 40px computed font-size (16 * 800/320), since Text Autosizing is not yet smart enough to detect that all the children of this div are unwrappable. Ideally it would not get autosized.
+</span>
+</div>
+
+<div>
+<span style="white-space: pre-wrap">
+    This text should be autosized to 40px computed font-size (16 * 800/320) since it has white-space:pre-wrap which allows text wrapping (even though it doesn't collapse white space).
+</span>
+</div>
+
+<div>
+<span>
+    This text should be autosized to 40px computed font-size (16 * 800/320).
+</span>
+</div>
+
+</body>
+</html>
index 729a28f..7d6f5db 100644 (file)
@@ -1,3 +1,31 @@
+2012-12-14  John Mellor  <johnme@chromium.org>
+
+        Text Autosizing: Don't autosize unwrappable blocks
+        https://bugs.webkit.org/show_bug.cgi?id=104925
+
+        Reviewed by Julien Chaffraix.
+
+        If we autosize an unwrappable block (white-space:nowrap/pre), it'll
+        expand sideways. This doesn't actually improve its legibility, and it
+        can often severely break web page layouts. This patch prevents us from
+        autosizing unwrappable blocks. A follow-up patch will address the more
+        complex issue of unwrappable inline elements.
+
+        Tests: fast/text-autosizing/unwrappable-blocks.html
+               fast/text-autosizing/unwrappable-inlines.html
+
+        * rendering/TextAutosizer.cpp:
+        (WebCore::TextAutosizer::processContainer):
+            Use containerShouldbeAutosized instead of contentHeightIsConstrained.
+        (WebCore::contentHeightIsConstrained):
+            Unchanged, just moved lower down the file.
+        (WebCore::TextAutosizer::containerShouldbeAutosized):
+            Checks that the block is wrappable, and also that contentHeightIsConstrained is false.
+        (WebCore::TextAutosizer::measureDescendantTextWidth):
+            Use containerShouldbeAutosized instead of contentHeightIsConstrained.
+        * rendering/TextAutosizer.h:
+            Declared containerShouldbeAutosized.
+
 2012-12-14  Vsevolod Vlasov  <vsevik@chromium.org>
 
         Web Inspector: Duplicate scripts appear in workspace when script was referenced by url with a fragment part.
index b102b2b..8b7f824 100644 (file)
@@ -119,31 +119,11 @@ void TextAutosizer::processCluster(RenderBlock* cluster, RenderBlock* container,
     processContainer(multiplier, container, subtreeRoot, windowInfo);
 }
 
-static bool contentHeightIsConstrained(const RenderBlock* container)
-{
-    // FIXME: Propagate constrainedness down the tree, to avoid inefficiently walking back up from each box.
-    // FIXME: This code needs to take into account vertical writing modes.
-    // FIXME: Consider additional heuristics, such as ignoring fixed heights if the content is already overflowing before autosizing kicks in.
-    for (; container; container = container->containingBlock()) {
-        RenderStyle* style = container->style();
-        if (style->overflowY() >= OSCROLL)
-            return false;
-        if (style->height().isSpecified() || style->maxHeight().isSpecified()) {
-            // Some sites (e.g. wikipedia) set their html and/or body elements to height:100%,
-            // without intending to constrain the height of the content within them.
-            return !container->isRoot() && !container->isBody();
-        }
-        if (container->isFloatingOrOutOfFlowPositioned())
-            return false;
-    }
-    return false;
-}
-
 void TextAutosizer::processContainer(float multiplier, RenderBlock* container, RenderObject* subtreeRoot, const TextAutosizingWindowInfo& windowInfo)
 {
     ASSERT(isAutosizingContainer(container));
 
-    float localMultiplier = contentHeightIsConstrained(container) ? 1 : multiplier;
+    float localMultiplier = containerShouldbeAutosized(container) ? multiplier: 1;
 
     RenderObject* descendant = nextInPreOrderSkippingDescendantsOfContainers(subtreeRoot, subtreeRoot);
     while (descendant) {
@@ -250,6 +230,36 @@ bool TextAutosizer::isAutosizingCluster(const RenderBlock* renderer)
     // renderers, and probably flexboxes...
 }
 
+static bool contentHeightIsConstrained(const RenderBlock* container)
+{
+    // FIXME: Propagate constrainedness down the tree, to avoid inefficiently walking back up from each box.
+    // FIXME: This code needs to take into account vertical writing modes.
+    // FIXME: Consider additional heuristics, such as ignoring fixed heights if the content is already overflowing before autosizing kicks in.
+    for (; container; container = container->containingBlock()) {
+        RenderStyle* style = container->style();
+        if (style->overflowY() >= OSCROLL)
+            return false;
+        if (style->height().isSpecified() || style->maxHeight().isSpecified()) {
+            // Some sites (e.g. wikipedia) set their html and/or body elements to height:100%,
+            // without intending to constrain the height of the content within them.
+            return !container->isRoot() && !container->isBody();
+        }
+        if (container->isFloatingOrOutOfFlowPositioned())
+            return false;
+    }
+    return false;
+}
+
+bool TextAutosizer::containerShouldbeAutosized(const RenderBlock* container)
+{
+    // Don't autosize block-level text that can't wrap (as it's likely to
+    // expand sideways and break the page's layout).
+    if (!container->style()->autoWrap())
+        return false;
+
+    return !contentHeightIsConstrained(container);
+}
+
 bool TextAutosizer::clusterShouldBeAutosized(const RenderBlock* lowestCommonAncestor, float commonAncestorWidth)
 {
     // Don't autosize clusters that contain less than 4 lines of text (in
@@ -273,7 +283,7 @@ bool TextAutosizer::clusterShouldBeAutosized(const RenderBlock* lowestCommonAnce
 
 void TextAutosizer::measureDescendantTextWidth(const RenderBlock* container, float minTextWidth, float& textWidth)
 {
-    bool skipLocalText = contentHeightIsConstrained(container);
+    bool skipLocalText = !containerShouldbeAutosized(container);
 
     RenderObject* descendant = nextInPreOrderSkippingDescendantsOfContainers(container, container);
     while (descendant) {
index a709b11..dd3e19d 100644 (file)
@@ -70,6 +70,7 @@ private:
     static bool isAutosizingContainer(const RenderObject*);
     static bool isAutosizingCluster(const RenderBlock*);
 
+    static bool containerShouldbeAutosized(const RenderBlock* container);
     static bool clusterShouldBeAutosized(const RenderBlock* lowestCommonAncestor, float commonAncestorWidth);
     static void measureDescendantTextWidth(const RenderBlock* container, float minTextWidth, float& textWidth);