min-content gets the wrong value if min-width is set on some form controls
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Jan 2013 19:41:01 +0000 (19:41 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Jan 2013 19:41:01 +0000 (19:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=106389

Reviewed by Tony Chang.

Source/WebCore:

Simplify the logic. The only exposed change in behavior is that
m_minPreferredLogicalWidth gets set to m_maxPreferredLogicalWidth
instead of 0 when min-width is set.

Test: fast/forms/min-content-form-controls.html

* rendering/RenderFileUploadControl.cpp:
(WebCore::RenderFileUploadControl::computePreferredLogicalWidths):
* rendering/RenderListBox.cpp:
(WebCore::RenderListBox::computePreferredLogicalWidths):
* rendering/RenderMenuList.cpp:
(WebCore::RenderMenuList::computePreferredLogicalWidths):
* rendering/RenderSlider.cpp:
(WebCore::RenderSlider::computePreferredLogicalWidths):
* rendering/RenderTextControl.cpp:
(WebCore::RenderTextControl::computePreferredLogicalWidths):

LayoutTests:

* fast/forms/min-content-form-controls-expected.txt: Added.
* fast/forms/min-content-form-controls.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/min-content-form-controls-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/min-content-form-controls.html [new file with mode: 0644]
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFileUploadControl.cpp
Source/WebCore/rendering/RenderListBox.cpp
Source/WebCore/rendering/RenderMenuList.cpp
Source/WebCore/rendering/RenderSlider.cpp
Source/WebCore/rendering/RenderTextControl.cpp

index af6c1b8..24a7d0b 100644 (file)
@@ -1,3 +1,13 @@
+2013-01-08  Ojan Vafai  <ojan@chromium.org>
+
+        min-content gets the wrong value if min-width is set on some form controls
+        https://bugs.webkit.org/show_bug.cgi?id=106389
+
+        Reviewed by Tony Chang.
+
+        * fast/forms/min-content-form-controls-expected.txt: Added.
+        * fast/forms/min-content-form-controls.html: Added.
+
 2013-01-09  Roger Fong  <roger_fong@apple.com>
 
         Unreviewed. Skipping 4 video controls tests on Windows.
diff --git a/LayoutTests/fast/forms/min-content-form-controls-expected.txt b/LayoutTests/fast/forms/min-content-form-controls-expected.txt
new file mode 100644 (file)
index 0000000..121bd25
--- /dev/null
@@ -0,0 +1,11 @@
+PASS document.getElementById("text-a").offsetWidth is document.getElementById("text-b").offsetWidth
+PASS document.getElementById("file-a").offsetWidth is document.getElementById("file-b").offsetWidth
+PASS document.getElementById("range-a").offsetWidth is document.getElementById("range-b").offsetWidth
+PASS document.getElementById("select-a").offsetWidth is document.getElementById("select-b").offsetWidth
+PASS document.getElementById("multiple-a").offsetWidth is document.getElementById("multiple-b").offsetWidth
+PASS successfullyParsed is true
+
+TEST COMPLETE
+The form controls of the same type below should be the same width.
+
+             
diff --git a/LayoutTests/fast/forms/min-content-form-controls.html b/LayoutTests/fast/forms/min-content-form-controls.html
new file mode 100644 (file)
index 0000000..1e767b1
--- /dev/null
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<style>
+.min-content {
+    min-width: 10px;
+    width: -webkit-min-content;
+    width: -moz-min-content;
+    width: -ie-min-content;
+    width: -o-min-content;
+    width: min-content;
+}
+</style>
+<p>The form controls of the same type below should be the same width.</p>
+
+<input id="text-a" class="min-content">
+<input id="text-b">
+
+<input id="file-a" type=file class="min-content">
+<input id="file-b" type=file>
+
+<input id="range-a" type=range class="min-content">
+<input id="range-b" type=range>
+
+<select id="select-a" class="min-content"><option>aaaaaaaaaaaaaaaaaaaaaaaaa</option></select>
+<select id="select-b"><option>aaaaaaaaaaaaaaaaaaaaaaaaa</option></select>
+
+<select id="multiple-a" multiple class="min-content"><option>aaaaaaaaaaaaaaaaaaaaaaaaa</option></select>
+<select id="multiple-b" multiple><option>aaaaaaaaaaaaaaaaaaaaaaaaa</option></select>
+
+<script src="../js/resources/js-test-pre.js"></script>
+<script>
+var ids = ['text', 'file', 'range', 'select', 'multiple'];
+ids.forEach(function(id) {
+    shouldBe('document.getElementById("' + id + '-a").offsetWidth', 'document.getElementById("' + id + '-b").offsetWidth');
+})
+</script>
+<script src="../js/resources/js-test-post.js"></script>
index ad1d78d..c43406a 100644 (file)
@@ -1244,6 +1244,10 @@ webkit.org/b/105777 platform/mac/editing/spelling/autocorrection-in-textarea.htm
 
 webkit.org/b/105932 [ MountainLion Debug ] http/tests/inspector/resource-har-pages.html [ Crash ]
 
+# Need to see this run on the bots to see what's wrong. The code change in 
+# https://bugs.webkit.org/show_bug.cgi?id=106389 is only touching cross-platform code.
+Bug(ojan) fast/forms/min-content-form-controls.html [ Failure ]
+
 webkit.org/b/105986 [ Debug ] transitions/hang-with-bad-transition-list.html [ Pass Crash ]
 webkit.org/b/105986 [ Debug ] svg/dynamic-updates/SVGFEMorphologyElement-dom-in-attr.html [ Pass Crash ]
 webkit.org/b/105986 [ Debug ] svg/dynamic-updates/SVGFEMorphologyElement-dom-radius-attr.html [ Pass Crash ]
index a0e9a7c..e4dd849 100644 (file)
@@ -1,3 +1,27 @@
+2013-01-08  Ojan Vafai  <ojan@chromium.org>
+
+        min-content gets the wrong value if min-width is set on some form controls
+        https://bugs.webkit.org/show_bug.cgi?id=106389
+
+        Reviewed by Tony Chang.
+
+        Simplify the logic. The only exposed change in behavior is that
+        m_minPreferredLogicalWidth gets set to m_maxPreferredLogicalWidth
+        instead of 0 when min-width is set.
+
+        Test: fast/forms/min-content-form-controls.html
+
+        * rendering/RenderFileUploadControl.cpp:
+        (WebCore::RenderFileUploadControl::computePreferredLogicalWidths):
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::computePreferredLogicalWidths):
+        * rendering/RenderMenuList.cpp:
+        (WebCore::RenderMenuList::computePreferredLogicalWidths):
+        * rendering/RenderSlider.cpp:
+        (WebCore::RenderSlider::computePreferredLogicalWidths):
+        * rendering/RenderTextControl.cpp:
+        (WebCore::RenderTextControl::computePreferredLogicalWidths):
+
 2013-01-09  Abhishek Arya  <inferno@chromium.org>
 
         Mitigate out-of-bounds access in InlineIterator
index 5b00ca2..da08493 100644 (file)
@@ -190,15 +190,15 @@ void RenderFileUploadControl::computePreferredLogicalWidths()
             if (RenderObject* buttonRenderer = button->renderer())
                 defaultLabelWidth += buttonRenderer->maxPreferredLogicalWidth() + afterButtonSpacing;
         m_maxPreferredLogicalWidth = static_cast<int>(ceilf(max(minDefaultLabelWidth, defaultLabelWidth)));
+
+        if (!style->width().isPercent())
+            m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
     }
 
     if (style->minWidth().isFixed() && style->minWidth().value() > 0) {
         m_maxPreferredLogicalWidth = max(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style->minWidth().value()));
         m_minPreferredLogicalWidth = max(m_minPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style->minWidth().value()));
-    } else if (style->width().isPercent())
-        m_minPreferredLogicalWidth = 0;
-    else
-        m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
+    }
 
     if (style->maxWidth().isFixed()) {
         m_maxPreferredLogicalWidth = min(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style->maxWidth().value()));
index 14ae0be..7139aac 100644 (file)
@@ -216,15 +216,15 @@ void RenderListBox::computePreferredLogicalWidths()
         m_maxPreferredLogicalWidth = m_optionsWidth + 2 * optionsSpacingHorizontal;
         if (m_vBar)
             m_maxPreferredLogicalWidth += m_vBar->width();
+
+        if (!style()->width().isPercent())
+            m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
     }
 
     if (style()->minWidth().isFixed() && style()->minWidth().value() > 0) {
         m_maxPreferredLogicalWidth = max(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->minWidth().value()));
         m_minPreferredLogicalWidth = max(m_minPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->minWidth().value()));
-    } else if (style()->width().isPercent())
-        m_minPreferredLogicalWidth = 0;
-    else
-        m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
+    }
 
     if (style()->maxWidth().isFixed()) {
         m_maxPreferredLogicalWidth = min(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->maxWidth().value()));
index 9f83e84..e8fd73d 100644 (file)
@@ -279,16 +279,17 @@ void RenderMenuList::computePreferredLogicalWidths()
     
     if (style()->width().isFixed() && style()->width().value() > 0)
         m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth = adjustContentBoxLogicalWidthForBoxSizing(style()->width().value());
-    else
+    else {
         m_maxPreferredLogicalWidth = max(m_optionsWidth, theme()->minimumMenuListSize(style())) + m_innerBlock->paddingLeft() + m_innerBlock->paddingRight();
 
+        if (!style()->width().isPercent())
+            m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
+    }
+
     if (style()->minWidth().isFixed() && style()->minWidth().value() > 0) {
         m_maxPreferredLogicalWidth = max(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->minWidth().value()));
         m_minPreferredLogicalWidth = max(m_minPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->minWidth().value()));
-    } else if (style()->width().isPercent())
-        m_minPreferredLogicalWidth = 0;
-    else
-        m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
+    }
 
     if (style()->maxWidth().isFixed()) {
         m_maxPreferredLogicalWidth = min(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->maxWidth().value()));
index 09a9a9c..ed92aff 100644 (file)
@@ -77,17 +77,18 @@ void RenderSlider::computePreferredLogicalWidths()
 
     if (style()->width().isFixed() && style()->width().value() > 0)
         m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth = adjustContentBoxLogicalWidthForBoxSizing(style()->width().value());
-    else
+    else {
         m_maxPreferredLogicalWidth = defaultTrackLength * style()->effectiveZoom();
 
+        if (!style()->width().isPercent())
+            m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
+    }
+
     if (style()->minWidth().isFixed() && style()->minWidth().value() > 0) {
         m_maxPreferredLogicalWidth = max(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->minWidth().value()));
         m_minPreferredLogicalWidth = max(m_minPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->minWidth().value()));
-    } else if (style()->width().isPercent())
-        m_minPreferredLogicalWidth = 0;
-    else
-        m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
-    
+    }
+
     if (style()->maxWidth().isFixed()) {
         m_maxPreferredLogicalWidth = min(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->maxWidth().value()));
         m_minPreferredLogicalWidth = min(m_minPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->maxWidth().value()));
index 761d68e..7043896 100644 (file)
@@ -269,15 +269,15 @@ void RenderTextControl::computePreferredLogicalWidths()
         m_maxPreferredLogicalWidth = preferredContentWidth(getAvgCharWidth(family));
         if (RenderBox* innerTextRenderBox = innerTextElement()->renderBox())
             m_maxPreferredLogicalWidth += innerTextRenderBox->paddingLeft() + innerTextRenderBox->paddingRight();
+
+        if (!style()->width().isPercent())
+            m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
     }
 
     if (style()->minWidth().isFixed() && style()->minWidth().value() > 0) {
         m_maxPreferredLogicalWidth = max(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->minWidth().value()));
         m_minPreferredLogicalWidth = max(m_minPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->minWidth().value()));
-    } else if (style()->width().isPercent())
-        m_minPreferredLogicalWidth = 0;
-    else
-        m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
+    }
 
     if (style()->maxWidth().isFixed()) {
         m_maxPreferredLogicalWidth = min(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(style()->maxWidth().value()));