Text Autosizing: prevent oscillation of font sizes during autosizing
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Feb 2013 18:57:31 +0000 (18:57 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Feb 2013 18:57:31 +0000 (18:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=108205

Patch by Tim Volodine <timvolodine@chromium.org> on 2013-02-19
Reviewed by Kenneth Rohde Christiansen.

Source/WebCore:

On some websites autosized font-sizes oscillate due to layouts caused by
hovering or incremental page loading (and on other sites font sizes do
eventually stabilize, but it takes many layouts before they reach a steady
size). To prevent all these cases, we no longer allow the autosizing
multiplier to change after it has been set (to a value other than 1).

This won't always give exactly the same results, but testing on 2000 top
sites shows that this makes little difference in practice, and it prevents
these very jarring cases. As a happy side-effect, this speeds up layouts
as font sizes change less.

Test: fast/text-autosizing/oscillation-javascript-fontsize-change.html

* page/FrameView.cpp:
(WebCore::FrameView::setFrameRect):
* page/Settings.cpp:
(WebCore::Settings::setTextAutosizingFontScaleFactor):
* rendering/TextAutosizer.cpp:
(WebCore::TextAutosizer::recalculateMultipliers):
(WebCore):
(WebCore::TextAutosizer::processContainer):
* rendering/TextAutosizer.h:
(TextAutosizer):

LayoutTests:

Added overflow-y:hidden to some existing tests, since previously those tests
would start off with incorrect multipliers (because mainFrame->view()-layoutSize()
is initially 785 instead of 800 as ScrollView wrongly guesses a scrollbar will
be needed), and then the multipliers would get corrected on a subsequent layout.
Now that we don't allow the multiplier to change after being set, it needs to be
right first time.
Also added specific oscillation test triggered by javascript.

* fast/text-autosizing/constrained-height-body-expected.html:
* fast/text-autosizing/constrained-height-body.html:
* fast/text-autosizing/constrained-then-float-ancestors-expected.html:
* fast/text-autosizing/constrained-then-float-ancestors.html:
* fast/text-autosizing/constrained-then-position-fixed-ancestors-expected.html:
* fast/text-autosizing/constrained-then-position-fixed-ancestors.html:
* fast/text-autosizing/nested-em-line-height-expected.html:
* fast/text-autosizing/nested-em-line-height.html:
* fast/text-autosizing/oscillation-javascript-fontsize-change-expected.html: Added.
* fast/text-autosizing/oscillation-javascript-fontsize-change.html: Added.
* fast/text-autosizing/simple-paragraph-expected.html:
* fast/text-autosizing/simple-paragraph.html:
* fast/text-autosizing/span-child-expected.html:
* fast/text-autosizing/span-child.html:
* fast/text-autosizing/unwrappable-blocks-expected.html:
* fast/text-autosizing/unwrappable-blocks.html:
* fast/text-autosizing/unwrappable-inlines-expected.html:
* fast/text-autosizing/unwrappable-inlines.html:

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

24 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/text-autosizing/constrained-height-body-expected.html
LayoutTests/fast/text-autosizing/constrained-height-body.html
LayoutTests/fast/text-autosizing/constrained-then-float-ancestors-expected.html
LayoutTests/fast/text-autosizing/constrained-then-float-ancestors.html
LayoutTests/fast/text-autosizing/constrained-then-position-fixed-ancestors-expected.html
LayoutTests/fast/text-autosizing/constrained-then-position-fixed-ancestors.html
LayoutTests/fast/text-autosizing/nested-em-line-height-expected.html
LayoutTests/fast/text-autosizing/nested-em-line-height.html
LayoutTests/fast/text-autosizing/oscillation-javascript-fontsize-change-expected.html [new file with mode: 0644]
LayoutTests/fast/text-autosizing/oscillation-javascript-fontsize-change.html [new file with mode: 0644]
LayoutTests/fast/text-autosizing/simple-paragraph-expected.html
LayoutTests/fast/text-autosizing/simple-paragraph.html
LayoutTests/fast/text-autosizing/span-child-expected.html
LayoutTests/fast/text-autosizing/span-child.html
LayoutTests/fast/text-autosizing/unwrappable-blocks-expected.html
LayoutTests/fast/text-autosizing/unwrappable-blocks.html
LayoutTests/fast/text-autosizing/unwrappable-inlines-expected.html
LayoutTests/fast/text-autosizing/unwrappable-inlines.html
Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/Settings.cpp
Source/WebCore/rendering/TextAutosizer.cpp
Source/WebCore/rendering/TextAutosizer.h

index 380de46184cddff5bfd1d5dcde8a4a8bd01fd21d..e72f1ebb1f7ff665851dbf58b71c92c76b03dd1d 100644 (file)
@@ -1,3 +1,37 @@
+2013-02-19  Tim Volodine  <timvolodine@chromium.org>
+
+        Text Autosizing: prevent oscillation of font sizes during autosizing
+        https://bugs.webkit.org/show_bug.cgi?id=108205
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        Added overflow-y:hidden to some existing tests, since previously those tests
+        would start off with incorrect multipliers (because mainFrame->view()-layoutSize()
+        is initially 785 instead of 800 as ScrollView wrongly guesses a scrollbar will
+        be needed), and then the multipliers would get corrected on a subsequent layout.
+        Now that we don't allow the multiplier to change after being set, it needs to be
+        right first time.
+        Also added specific oscillation test triggered by javascript.
+
+        * fast/text-autosizing/constrained-height-body-expected.html:
+        * fast/text-autosizing/constrained-height-body.html:
+        * fast/text-autosizing/constrained-then-float-ancestors-expected.html:
+        * fast/text-autosizing/constrained-then-float-ancestors.html:
+        * fast/text-autosizing/constrained-then-position-fixed-ancestors-expected.html:
+        * fast/text-autosizing/constrained-then-position-fixed-ancestors.html:
+        * fast/text-autosizing/nested-em-line-height-expected.html:
+        * fast/text-autosizing/nested-em-line-height.html:
+        * fast/text-autosizing/oscillation-javascript-fontsize-change-expected.html: Added.
+        * fast/text-autosizing/oscillation-javascript-fontsize-change.html: Added.
+        * fast/text-autosizing/simple-paragraph-expected.html:
+        * fast/text-autosizing/simple-paragraph.html:
+        * fast/text-autosizing/span-child-expected.html:
+        * fast/text-autosizing/span-child.html:
+        * fast/text-autosizing/unwrappable-blocks-expected.html:
+        * fast/text-autosizing/unwrappable-blocks.html:
+        * fast/text-autosizing/unwrappable-inlines-expected.html:
+        * fast/text-autosizing/unwrappable-inlines.html:
+
 2013-02-19  Youenn Fablet  <youennf@gmail.com>
 
         [EFL][DRT] http/tests/loading/307-after-303-after-post.html times out
index 59a1080c835d21bfadb1beca0f96c1505e7a20c2..be3cd2f386284ac8fcfa57b88193fbf3b1731c3c 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 </head>
index 26ec70a2b9203d3d9207300d31fecd94d5077781..bc41fbdc6bf12f79d6d4ba1fb9a33ef2f0046c34 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 <script>
index 1e6710733eacf2c7c0a34a48ad0291e02fd819c0..a7e662617a4ed430bfa275ae72fedb87c7c438f3 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 </head>
index 83c9bb2f360629000cf29ee242113bd040ec48b0..b82d185778d46dd227e47afcbed7a33063383d18 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 <script>
index 783a512a4e2f8b2291561a3f1d8444d541b6b9a4..3b24b4e4eb0529424f38e36ff9cc464fd8e484db 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 </head>
index 24c77d4d8dc7cb16d748617538965002dfcda5ee..06383d5d5d54e0ce205ea62ad845f446fc7d97f7 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 <script>
index caffb6849ce835190ab30a117b32128664c0d968..32638c45c097bd18d9a01da673338c1c38d8372c 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 </head>
index 2baa55ea9c21f91ac7f541b8c421ac34c80ee29b..2aa345edbd18d67f179293764d7e2c072622a8ec 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 <script>
diff --git a/LayoutTests/fast/text-autosizing/oscillation-javascript-fontsize-change-expected.html b/LayoutTests/fast/text-autosizing/oscillation-javascript-fontsize-change-expected.html
new file mode 100644 (file)
index 0000000..04a7946
--- /dev/null
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html style="font-size: 16px">
+<head>
+
+<meta name="viewport" content="width=800">
+<style>
+  body {
+    width: 800px;
+    margin: 0;
+    overflow-y: hidden;
+  }
+</style>
+
+</head>
+<body>
+
+<div style="top:50px;position:absolute;font-size:19.8px">
+    . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .<br/>
+    This paragraph should be autosized to 19.8px<br/>
+    because it contains line breaks.<br/>
+    This test is intended to check<br/>
+    that there are no oscillations due to javascript<br/>
+    briefly increasing the font size of a<br/>
+    small paragraph below.
+</div>
+<div id="sizechanging">
+    This text changes size using javascript below.
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/text-autosizing/oscillation-javascript-fontsize-change.html b/LayoutTests/fast/text-autosizing/oscillation-javascript-fontsize-change.html
new file mode 100644 (file)
index 0000000..190cce4
--- /dev/null
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html style="font-size: 16px">
+<head>
+
+<meta name="viewport" content="width=800">
+<style>
+  body {
+    width: 800px;
+    margin: 0;
+    overflow-y: hidden;
+  }
+  .largersize{font-size: 1.1em}
+</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 style="top:50px;position:absolute;">
+    . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .<br/>
+    This paragraph should be autosized to 19.8px<br/>
+    because it contains line breaks.<br/>
+    This test is intended to check<br/>
+    that there are no oscillations due to javascript<br/>
+    briefly increasing the font size of a<br/>
+    small paragraph below.
+</div>
+<div id="sizechanging">
+    This text changes size using javascript below.
+</div>
+
+<script>
+element = document.getElementById("sizechanging");
+if (element.offsetHeight) {
+    // force layout (computation of offsetHeight triggers reflow)
+}
+element.className = 'largersize';
+if (element.offsetHeight) {}
+element.className = '';
+if (element.offsetHeight) {}
+</script>
+<noscript>fail (no support for javascript)</noscript>
+
+</body>
+</html>
index 3fd4a3a4774caaaa25d30301a1f792881f674d65..d15ab21207ae6f8f7215ccb04dacc81dc5742818 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 </head>
index 850916e877f50502e9379fefbc78898dd8a9270e..c974c4b1d52af4908b88671cc6209d09d012e7b8 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 <script>
index cc442163f37d6aa1351afd865ab489653ae66bb8..c02991a768b294c32ff34cd781767638e2d27c90 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 </head>
index 825bf44fa719acd404288a7b6e57801b02004e3f..7754063567dbae3cedcd09519c21a98d9bfbf602 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 </style>
 
 <script>
index 48d3b888b179ac2fdfa2dd05875065cacd2b8f1c..7ed18dfaab72e4e3a9231ee774c52738df58fb7a 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 pre { margin: 0; }
 </style>
 
index b6603aed447c9c1686182222eaa48552ee1556c5..792945496cac106b65df97075bc2f778c87c5e8e 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 pre { margin: 0; }
 </style>
 
index 48fc903908df2abbfe48fbca7262c51b880bd6ee..a7188141d901288ab498bb56042a12c4d62249c7 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 pre { margin: 0; }
 </style>
 
index 9b0732ee86cd3ba96049acf9e06a317292c6dbe1..16434c98f75a0540b5b7dd32a17a3554a94eb897 100644 (file)
@@ -5,7 +5,7 @@
 <meta name="viewport" content="width=800">
 <style>
 html { font-size: 16px; }
-body { width: 800px; margin: 0; }
+body { width: 800px; margin: 0; overflow-y: hidden; }
 pre { margin: 0; }
 </style>
 
index b485f425fbc3d31d9a2a006a256799a6a1d899e3..9226af16cf9ff694e1dc16852571b1161318167c 100644 (file)
@@ -1,3 +1,34 @@
+2013-02-19  Tim Volodine  <timvolodine@chromium.org>
+
+        Text Autosizing: prevent oscillation of font sizes during autosizing
+        https://bugs.webkit.org/show_bug.cgi?id=108205
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        On some websites autosized font-sizes oscillate due to layouts caused by
+        hovering or incremental page loading (and on other sites font sizes do
+        eventually stabilize, but it takes many layouts before they reach a steady
+        size). To prevent all these cases, we no longer allow the autosizing
+        multiplier to change after it has been set (to a value other than 1).
+
+        This won't always give exactly the same results, but testing on 2000 top
+        sites shows that this makes little difference in practice, and it prevents
+        these very jarring cases. As a happy side-effect, this speeds up layouts
+        as font sizes change less.
+
+        Test: fast/text-autosizing/oscillation-javascript-fontsize-change.html
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::setFrameRect):
+        * page/Settings.cpp:
+        (WebCore::Settings::setTextAutosizingFontScaleFactor):
+        * rendering/TextAutosizer.cpp:
+        (WebCore::TextAutosizer::recalculateMultipliers):
+        (WebCore):
+        (WebCore::TextAutosizer::processContainer):
+        * rendering/TextAutosizer.h:
+        (TextAutosizer):
+
 2013-02-19  Youenn Fablet  <youennf@gmail.com>
 
         [EFL][DRT] http/tests/loading/307-after-303-after-post.html times out
index 06f2889993e0720584fc91bb85705be0fe729880..6b6b71fa392679a1386bd34efdac7486635514ea 100644 (file)
@@ -444,6 +444,17 @@ void FrameView::setFrameRect(const IntRect& newRect)
     if (newRect == oldRect)
         return;
 
+#if ENABLE(TEXT_AUTOSIZING)
+    // Autosized font sizes depend on the width of the viewing area.
+    if (newRect.width() != oldRect.width()) {
+        Page* page = m_frame ? m_frame->page() : 0;
+        if (page && page->mainFrame() == m_frame && page->settings()->textAutosizingEnabled()) {
+            for (Frame* frame = page->mainFrame(); frame; frame = frame->tree()->traverseNext())
+                m_frame->document()->textAutosizer()->recalculateMultipliers();
+        }
+    }
+#endif
+
     ScrollView::setFrameRect(newRect);
 
     updateScrollableAreaSet();
index ffcda345e6e45d8e0916934a424ed648a80cddf2..e88a075d6100bd8038f6213548c09d0673f4d5e1 100644 (file)
@@ -42,6 +42,7 @@
 #include "PageCache.h"
 #include "ResourceHandle.h"
 #include "StorageMap.h"
+#include "TextAutosizer.h"
 #include <limits>
 
 using namespace std;
@@ -364,6 +365,11 @@ void Settings::setTextAutosizingWindowSizeOverride(const IntSize& textAutosizing
 void Settings::setTextAutosizingFontScaleFactor(float fontScaleFactor)
 {
     m_textAutosizingFontScaleFactor = fontScaleFactor;
+
+    // FIXME: I wonder if this needs to traverse frames like in WebViewImpl::resize, or whether there is only one document per Settings instance?
+    for (Frame* frame = m_page->mainFrame(); frame; frame = frame->tree()->traverseNext())
+        frame->document()->textAutosizer()->recalculateMultipliers();
+
     m_page->setNeedsRecalcStyleInAllFrames();
 }
 
index 626a772b5b0789aa1c4518bd0b692e8771032d59..6c926b804cd0cf189d4eafc0f83da81f365838f8 100644 (file)
@@ -91,6 +91,16 @@ TextAutosizer::~TextAutosizer()
 {
 }
 
+void TextAutosizer::recalculateMultipliers()
+{
+    RenderObject* renderer = m_document->renderer();
+    while (renderer) {
+        if (renderer->style() && renderer->style()->textAutosizingMultiplier() != 1)
+            setMultiplier(renderer, 1);
+        renderer = renderer->nextInPreOrder();
+    }
+}
+
 bool TextAutosizer::processSubtree(RenderObject* layoutRoot)
 {
     // FIXME: Text Autosizing should only be enabled when m_document->page()->mainFrame()->view()->useFixedLayout()
@@ -189,7 +199,7 @@ void TextAutosizer::processContainer(float multiplier, RenderBlock* container, T
     RenderObject* descendant = nextInPreOrderSkippingDescendantsOfContainers(subtreeRoot, subtreeRoot);
     while (descendant) {
         if (descendant->isText()) {
-            if (localMultiplier != descendant->style()->textAutosizingMultiplier()) {
+            if (localMultiplier != 1 && descendant->style()->textAutosizingMultiplier() == 1) {
                 setMultiplier(descendant, localMultiplier);
                 setMultiplier(descendant->parent(), localMultiplier); // Parent does line spacing.
             }
index bb38baa86c7e6444fee73fefc7794e80fe2ec84f..1f7f7b711e0dc5c14fbd62c2693fded10afd2da3 100644 (file)
@@ -50,6 +50,7 @@ public:
     virtual ~TextAutosizer();
 
     bool processSubtree(RenderObject* layoutRoot);
+    void recalculateMultipliers();
 
     static float computeAutosizedFontSize(float specifiedSize, float multiplier);