Attachment 178577 Details for Bug 102560 – Patch

[patch] Patch

bug-102560-20121210175343.patch (text/plain), 15.42 KB, created by timvolodine on 2012-12-10 09:56:21 PST

(hide)

Description:

Filename:

MIME Type:

Creator: timvolodine

Created: 2012-12-10 09:56:21 PST

Size: 15.42 KB

patch

obsolete

>Subversion Revision: 136506
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 400b26105ec70e2639c67d0e9ab34e6fab28aed7..3c26afa940fbe941294558bff8101c6fa16c0004 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2012-12-10  Tim Volodine  <timvolodine@google.com>
>+
>+        Text Autosizing: Work out what to do about form controls
>+        https://bugs.webkit.org/show_bug.cgi?id=102560
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Dissalow boosting of form input controls like buttons and text field inputs. This is a short term solution,
>+        to avoid inconsistent boosting of form controls (which is worse than not boosting at all).
>+
>+        Test: fast/text-autosizing/form-controls-boosting.html
>+
>+        * rendering/TextAutosizer.cpp:
>+        (WebCore):
>+        (WebCore::TextAutosizer::processSubtree):
>+        (WebCore::TextAutosizer::processCluster):
>+        (WebCore::TextAutosizer::contentHeightIsConstrained):
>+        (WebCore::TextAutosizer::isAutosizingContainer):
>+        (WebCore::TextAutosizer::clusterShouldBeAutosized):
>+        (WebCore::TextAutosizer::containerContainsTags):
>+        (WebCore::TextAutosizer::findDeepestBlockContainingAllText):
>+        * rendering/TextAutosizer.h:
>+
> 2012-12-04  Antoine Quint  <graouts@apple.com>
> 
>         TextTrack's .kind property should return lower-case values
>diff --git a/Source/WebCore/rendering/TextAutosizer.cpp b/Source/WebCore/rendering/TextAutosizer.cpp
>index b102b2b5b29375c728492c1c984bf7f865d87221..dffaee53ab25b04dd5e071bd944e63652f60ba70 100644
>--- a/Source/WebCore/rendering/TextAutosizer.cpp
>+++ b/Source/WebCore/rendering/TextAutosizer.cpp
>@@ -25,6 +25,7 @@
> #include "TextAutosizer.h"
> 
> #include "Document.h"
>+#include "HTMLElement.h"
> #include "InspectorInstrumentation.h"
> #include "RenderObject.h"
> #include "RenderStyle.h"
>@@ -37,6 +38,8 @@
> 
> namespace WebCore {
> 
>+using namespace HTMLNames;
>+
> struct TextAutosizingWindowInfo {
>     IntSize windowSize;
>     IntSize minLayoutSize;
>@@ -89,6 +92,7 @@ bool TextAutosizer::processSubtree(RenderObject* layoutRoot)
>         cluster = cluster->containingBlock();
> 
>     processCluster(cluster, container, layoutRoot, windowInfo);
>+
>     return true;
> }
> 
>@@ -105,7 +109,7 @@ void TextAutosizer::processCluster(RenderBlock* cluster, RenderBlock* container,
>     float commonAncestorWidth = lowestCommonAncestor->contentLogicalWidth();
> 
>     float multiplier = 1;
>-    if (clusterShouldBeAutosized(lowestCommonAncestor, commonAncestorWidth)) {
>+    if (clusterShouldBeAutosized(cluster, lowestCommonAncestor, commonAncestorWidth)) {
>         int logicalWindowWidth = cluster->isHorizontalWritingMode() ? windowInfo.windowSize.width() : windowInfo.windowSize.height();
>         int logicalLayoutWidth = cluster->isHorizontalWritingMode() ? windowInfo.minLayoutSize.width() : windowInfo.minLayoutSize.height();
>         // Ignore box width in excess of the layout width, to avoid extreme multipliers.
>@@ -119,11 +123,15 @@ void TextAutosizer::processCluster(RenderBlock* cluster, RenderBlock* container,
>     processContainer(multiplier, container, subtreeRoot, windowInfo);
> }
> 
>-static bool contentHeightIsConstrained(const RenderBlock* container)
>+bool TextAutosizer::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.
>+    QualifiedName tags[4] = {inputTag, textareaTag, buttonTag, formTag};
>+    if (containerContainsTags(container, tags, 4))
>+        return true;
>+
>     for (; container; container = container->containingBlock()) {
>         RenderStyle* style = container->style();
>         if (style->overflowY() >= OSCROLL)
>@@ -211,6 +219,12 @@ bool TextAutosizer::isAutosizingContainer(const RenderObject* renderer)
>         return false;
>     if (renderer->isListItem())
>         return renderer->isFloating() || renderer->isOutOfFlowPositioned();
>+    // avoid creating containers for text within text controls or buttons
>+    if (renderer->parent()) {
>+        if (renderer->parent()->isRenderButton() || renderer->parent()->isTextControl())
>+            return false;
>+    }
>+
>     return true;
> }
> 
>@@ -250,7 +264,7 @@ bool TextAutosizer::isAutosizingCluster(const RenderBlock* renderer)
>     // renderers, and probably flexboxes...
> }
> 
>-bool TextAutosizer::clusterShouldBeAutosized(const RenderBlock* lowestCommonAncestor, float commonAncestorWidth)
>+bool TextAutosizer::clusterShouldBeAutosized(const RenderBlock* cluster, const RenderBlock* lowestCommonAncestor, float commonAncestorWidth)
> {
>     // Don't autosize clusters that contain less than 4 lines of text (in
>     // practice less lines are required, since measureDescendantTextWidth
>@@ -271,6 +285,32 @@ bool TextAutosizer::clusterShouldBeAutosized(const RenderBlock* lowestCommonAnce
>     return false;
> }
> 
>+bool TextAutosizer::containerContainsTags(const RenderObject* container, const QualifiedName tags[], const size_t numTags)
>+{
>+    const Node* containerNode = container->node();
>+    bool validCandidate = containerNode && containerNode->isHTMLElement() && !containerNode->isShadowRoot();
>+    if (validCandidate) {
>+        for (size_t i = 0; i < numTags; ++i) {
>+            if (containerNode->hasTagName(tags[i]))
>+                return true;
>+        }
>+    }
>+
>+    RenderObject* descendant = nextInPreOrderSkippingDescendantsOfContainers(container, container);
>+    while (descendant) {
>+        const Node* descendantNode = descendant->node();
>+        bool validCandidate = descendantNode && descendantNode->isHTMLElement() && !descendantNode->isShadowRoot();
>+        if (validCandidate) {
>+            for (size_t i = 0; i < numTags; ++i) {
>+                if (descendantNode->hasTagName(tags[i]))
>+                    return true;
>+            }
>+        }
>+        descendant = nextInPreOrderSkippingDescendantsOfContainers(descendant, container);
>+    }
>+    return false;
>+}
>+
> void TextAutosizer::measureDescendantTextWidth(const RenderBlock* container, float minTextWidth, float& textWidth)
> {
>     bool skipLocalText = contentHeightIsConstrained(container);
>@@ -319,7 +359,7 @@ const RenderBlock* TextAutosizer::findDeepestBlockContainingAllText(const Render
>     const RenderObject* lastTextLeaf = findFirstTextLeafNotInCluster(cluster, lastDepth, LastToFirst);
>     ASSERT(lastTextLeaf);
> 
>-    // Equalize the depths if necessary. Only one of the while loops below will get executed.
>+    // Equalize the depths if contentHeightIsConstrainednecessary. Only one of the while loops below will get executed.
>     const RenderObject* firstNode = firstTextLeaf;
>     const RenderObject* lastNode = lastTextLeaf;
>     while (firstDepth > lastDepth) {
>diff --git a/Source/WebCore/rendering/TextAutosizer.h b/Source/WebCore/rendering/TextAutosizer.h
>index a709b1168e44df00ec1f9d73bbe5e516fe926774..6cf744344e7c5242590b393b2d5b97ab3f0a3356 100644
>--- a/Source/WebCore/rendering/TextAutosizer.h
>+++ b/Source/WebCore/rendering/TextAutosizer.h
>@@ -28,6 +28,7 @@
> 
> #if ENABLE(TEXT_AUTOSIZING)
> 
>+#include "HTMLNames.h"
> #include "IntSize.h"
> #include <wtf/Noncopyable.h>
> #include <wtf/PassOwnPtr.h>
>@@ -70,7 +71,9 @@ private:
>     static bool isAutosizingContainer(const RenderObject*);
>     static bool isAutosizingCluster(const RenderBlock*);
> 
>-    static bool clusterShouldBeAutosized(const RenderBlock* lowestCommonAncestor, float commonAncestorWidth);
>+    static bool clusterShouldBeAutosized(const RenderBlock* cluster, const RenderBlock* lowestCommonAncestor, float commonAncestorWidth);
>+    static bool contentHeightIsConstrained(const RenderBlock* container);
>+    static bool containerContainsTags(const RenderObject* cluster, const QualifiedName tags[], const size_t numTags);
>     static void measureDescendantTextWidth(const RenderBlock* container, float minTextWidth, float& textWidth);
> 
>     // Use to traverse the tree of descendants, excluding descendants of containers (but returning the containers themselves).
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index a89dd45f4540b5c836f84c2f4d07df459719ed1f..48806cc9195f3d724c56cd339bc53ab6f8667317 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2012-12-10  Tim Volodine  <timvolodine@google.com>
>+
>+        Text Autosizing: Work out what to do about form controls
>+        https://bugs.webkit.org/show_bug.cgi?id=102560
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Dissalow boosting of form input elements like buttons and text input fields. This is a short term solution,
>+        to avoid inconsistent boosting of form controls (which is worse than not boosting).
>+
>+        * fast/text-autosizing/form-controls-boosting-expected.html: Added.
>+        * fast/text-autosizing/form-controls-boosting.html: Added.
>+
> 2012-12-04  Antoine Quint  <graouts@apple.com>
> 
>         TextTrack's .kind property should return lower-case values
>diff --git a/LayoutTests/fast/text-autosizing/form-controls-boosting-expected.html b/LayoutTests/fast/text-autosizing/form-controls-boosting-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..ced7e1f70294a6fbf5adea33a74f0f748159a376
>--- /dev/null
>+++ b/LayoutTests/fast/text-autosizing/form-controls-boosting-expected.html
>@@ -0,0 +1,114 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+
>+<meta name="viewport" content="width=800">
>+<style>
>+  body {
>+    width: 800px;
>+    margin: 0;
>+    overflow-y: hidden;
>+    font-family: Verdana, sans-serif;
>+    font-size: small;
>+  }
>+  
>+  #boosted {
>+    font-size: 2.05rem;
>+  }
>+  
>+  #header form {
>+    font-size: 85%;
>+    display: inline;
>+  }
>+  
>+  #header .links {
>+    font-size: 85%;
>+  }
>+  
>+  ul.links {
>+    margin: 0;
>+    padding: 0;
>+    list-style-type: none;
>+  }
>+
>+  ul.links li {
>+    display: inline;
>+    white-space: nowrap;
>+  }
>+</style>
>+
>+</head>
>+<body>
>+
>+<div id="header">
>+  <ul class="links">
>+    <li>
>+      <a href="./">Home</a>
>+    </li>
>+    <li>
>+      <span>| </span>
>+      <a href="">New</a>
>+    </li>
>+    <li>
>+      <span>| </span>
>+      <a href="">Search</a>
>+    </li>
>+    <li>
>+      <span>| </span>
>+      <form>
>+        <input class="txt" type="text" id="quicksearch_top" name="quicksearch">
>+        <input class="btn" type="submit" value="Find" id=find_top">
>+      </form>
>+    </li>
>+    <li>
>+      <span>| </span>
>+      <a href="">Reports</a>
>+    </li>
>+  </ul>
>+</div>
>+
>+<div>
>+  <p><div id="boosted">This test checks that from controls like buttons, text inputs and check-boxes are not autosized.
>+    The text in this paragraph should be autosized, while the form controls in the header and in the table-layout
>+    below should not. Also the "Find" button and text-input box at the bottom of the page should not be autosized.
>+  </div></p>
>+
>+  <form name="login" action="" method="POST">
>+  <table>
>+    <tr>
>+      <th align="right"><label for="Bugzilla_login">Login:</label></th>
>+      <td><input size="20" id="Bugzilla_login" name="Bugzilla_login"></td>
>+    </tr>
>+    <tr>
>+      <th align="right"><label for="Bugzilla_password">Password:</label></th>
>+      <td>
>+        <input type="password" size="20" id="Bugzilla_password" name="Bugzilla_password">
>+      </td>
>+    </tr>
>+    <tr>
>+      <th>&nbsp;</th>
>+      <td>
>+        <input type="checkbox" id="Bugzilla_restrictlogin" name="Bugzilla_restrictlogin" checked="checked">
>+        <label for="Bugzilla_restrictlogin">Restrict this session to this IP address</label>
>+      </td>
>+    </tr>
>+    <tr>
>+      <td><input type="submit" name="GoAheadAndLogIn" value="Login" id="log_in"></td>
>+      <td>[ <a href="">Forgot my Password</a> ]</td>
>+    </tr>
>+  </table>
>+  </form>
>+  
>+  <form>
>+  <div>
>+    <p><div id="boosted">This text should be autosized, while the input controls below should not.</div></p>
>+    <input id="quicksearch" type="text" name="quicksearch">
>+    <input id="find" type="submit" value="Find">
>+    <a href="">[Help]</a>
>+  </div>
>+  </form>
>+
>+</div>
>+
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/text-autosizing/form-controls-boosting.html b/LayoutTests/fast/text-autosizing/form-controls-boosting.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..85b0b5921f9c7e1a5fb115f8135b2712ea34b839
>--- /dev/null
>+++ b/LayoutTests/fast/text-autosizing/form-controls-boosting.html
>@@ -0,0 +1,119 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+
>+<meta name="viewport" content="width=800">
>+<style>
>+  body {
>+    width: 800px;
>+    margin: 0;
>+    overflow-y: hidden;
>+    font-family: Verdana, sans-serif;
>+    font-size: small;
>+  }
>+  
>+  #header form {
>+    font-size: 85%;
>+    display: inline;
>+  }
>+  
>+  #header .links {
>+    font-size: 85%;
>+  }
>+  
>+  ul.links {
>+    margin: 0;
>+    padding: 0;
>+    list-style-type: none;
>+  }
>+
>+  ul.links li {
>+    display: inline;
>+    white-space: nowrap;
>+  }
>+</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="header">
>+  <ul class="links">
>+    <li>
>+      <a href="./">Home</a>
>+    </li>
>+    <li>
>+      <span>| </span>
>+      <a href="">New</a>
>+    </li>
>+    <li>
>+      <span>| </span>
>+      <a href="">Search</a>
>+    </li>
>+    <li>
>+      <span>| </span>
>+      <form>
>+        <input class="txt" type="text" id="quicksearch_top" name="quicksearch">
>+        <input class="btn" type="submit" value="Find" id=find_top">
>+      </form>
>+    </li>
>+    <li>
>+      <span>| </span>
>+      <a href="">Reports</a>
>+    </li>
>+  </ul>
>+</div>
>+
>+<div>
>+  <p>This test checks that from controls like buttons, text inputs and check-boxes are not autosized.
>+    The text in this paragraph should be autosized, while the form controls in the header and in the table-layout
>+    below should not. Also the "Find" button and text-input box at the bottom of the page should not be autosized.
>+  </p>
>+
>+  <form name="login" action="" method="POST">
>+  <table>
>+    <tr>
>+      <th align="right"><label for="Bugzilla_login">Login:</label></th>
>+      <td><input size="20" id="Bugzilla_login" name="Bugzilla_login"></td>
>+    </tr>
>+    <tr>
>+      <th align="right"><label for="Bugzilla_password">Password:</label></th>
>+      <td>
>+        <input type="password" size="20" id="Bugzilla_password" name="Bugzilla_password">
>+      </td>
>+    </tr>
>+    <tr>
>+      <th>&nbsp;</th>
>+      <td>
>+        <input type="checkbox" id="Bugzilla_restrictlogin" name="Bugzilla_restrictlogin" checked="checked">
>+        <label for="Bugzilla_restrictlogin">Restrict this session to this IP address</label>
>+      </td>
>+    </tr>
>+    <tr>
>+      <td><input type="submit" name="GoAheadAndLogIn" value="Login" id="log_in"></td>
>+      <td>[ <a href="">Forgot my Password</a> ]</td>
>+    </tr>
>+  </table>
>+  </form>
>+  
>+  <form>
>+  <div>
>+    <p>This text should be autosized, while the input controls below should not.</p>
>+    <input id="quicksearch" type="text" name="quicksearch">
>+    <input id="find" type="submit" value="Find">
>+    <a href="">[Help]</a>
>+  </div>
>+  </form>
>+
>+</div>
>+
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 102560: 178577 | 178578 | 178787 | 179060 | 179285 | 179476 | 179492 | 179503 | 179774 | 179806 | 180137 | 180139