WebCore:
authoradele <adele@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Oct 2007 18:39:05 +0000 (18:39 +0000)
committeradele <adele@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Oct 2007 18:39:05 +0000 (18:39 +0000)
        Reviewed by Darin.

        Fix for http://bugs.webkit.org/show_bug.cgi?id=15252
        <rdar://problem/5498184> REGRESSION: <select multiple> doesn't scroll to top when old options are removed and new ones are added, leaving listbox empty-looking

        * rendering/RenderListBox.cpp: (WebCore::RenderListBox::calcHeight): If the scrollbar is disabled, make sure the scroll offset gets reset to 0.  In general,
          we don't want to unnecessarily adjust the scroll offset, but in this case, there won't be an obvious way for the user to adjust the scroller position once it's disabled.

LayoutTests:

        Reviewed by Darin.

        Test for http://bugs.webkit.org/show_bug.cgi?id=15252
        <rdar://problem/5498184> REGRESSION: <select multiple> doesn't scroll to top when old options are removed and new ones are added, leaving listbox empty-looking

        * fast/forms/listbox-scroll-after-options-removed-expected.txt: Added.
        * fast/forms/listbox-scroll-after-options-removed.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/listbox-scroll-after-options-removed-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/listbox-scroll-after-options-removed.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/rendering/RenderListBox.cpp

index 89820d3..5539205 100644 (file)
@@ -1,3 +1,13 @@
+2007-10-15  Adele Peterson  <adele@apple.com>
+
+        Reviewed by Darin.
+
+        Test for http://bugs.webkit.org/show_bug.cgi?id=15252
+        <rdar://problem/5498184> REGRESSION: <select multiple> doesn't scroll to top when old options are removed and new ones are added, leaving listbox empty-looking
+
+        * fast/forms/listbox-scroll-after-options-removed-expected.txt: Added.
+        * fast/forms/listbox-scroll-after-options-removed.html: Added.
+
 2007-10-15  Alice Liu  <alice.liu@apple.com>\r
 \r
         removing fixed test\r
diff --git a/LayoutTests/fast/forms/listbox-scroll-after-options-removed-expected.txt b/LayoutTests/fast/forms/listbox-scroll-after-options-removed-expected.txt
new file mode 100644 (file)
index 0000000..e6d96b7
--- /dev/null
@@ -0,0 +1,6 @@
+Bug 15252: <select multiple> doesn't scroll to top when old options are removed and new ones are added, leaving listbox empty-looking
+When the test runs, all the <option>s in the select are removed and replaced with 1 new option. If the bug is present, the select will appear empty until you scroll up with the mousewheel.
+
+
+Test Passed
+
diff --git a/LayoutTests/fast/forms/listbox-scroll-after-options-removed.html b/LayoutTests/fast/forms/listbox-scroll-after-options-removed.html
new file mode 100644 (file)
index 0000000..0953f3c
--- /dev/null
@@ -0,0 +1,55 @@
+<html>
+    <head>
+        <script>
+        function test()
+        {
+            if (window.layoutTestController) {
+                layoutTestController.dumpAsText();
+                layoutTestController.waitUntilDone();
+            }
+
+            setTimeout(test2, 0);
+        }
+        
+        function test2()
+        {
+            var select = document.getElementById("listbox");
+            while (select.hasChildNodes())
+                select.removeChild(select.firstChild);
+
+            var option = document.createElement("option");
+            option.innerText = "Test Passed";
+            select.appendChild(option);
+        
+            var scrollTop = select.scrollTop;
+            if (scrollTop == 0)
+                log("Test Passed");
+            else
+                log("Test Failed.  scrollTop = " + scrollTop + " even though there is only one option in the listbox");
+            
+            if (window.layoutTestController)
+                layoutTestController.notifyDone();
+        }
+        
+        function log(msg)
+        {
+            document.getElementById('console').appendChild(document.createTextNode(msg + '\n'));
+        }
+        </script>
+    </head>
+    <body onload="test()">
+        <a href="http://bugs.webkit.org/show_bug.cgi?id=15252">Bug 15252: &lt;select multiple&gt; doesn't scroll to top when old options are removed and new ones are added, leaving listbox empty-looking</a>
+
+        <p>When the test runs, all the &lt;option&gt;s in the select are removed and replaced with 1 new option. If the bug is present, the select will appear empty until you scroll up with the mousewheel.</p>
+
+        <select id="listbox" size="4">
+            <option>1</option>
+            <option>2</option>
+            <option>3</option>
+            <option>4</option>
+            <option>5</option>
+            <option selected>6</option>
+        </select>
+        <pre id="console"></pre>
+    </body>
+</html>
index aa6e7f4..a8d1928 100644 (file)
@@ -1,3 +1,13 @@
+2007-10-15  Adele Peterson  <adele@apple.com>
+
+        Reviewed by Darin.
+
+        Fix for http://bugs.webkit.org/show_bug.cgi?id=15252
+        <rdar://problem/5498184> REGRESSION: <select multiple> doesn't scroll to top when old options are removed and new ones are added, leaving listbox empty-looking
+
+        * rendering/RenderListBox.cpp: (WebCore::RenderListBox::calcHeight): If the scrollbar is disabled, make sure the scroll offset gets reset to 0.  In general,
+          we don't want to unnecessarily adjust the scroll offset, but in this case, there won't be an obvious way for the user to adjust the scroller position once it's disabled.
+
 2007-10-15  Jon Honeycutt  <jhoneycutt@apple.com>
 
         Reviewed by Maciej.
index 0da11e8..276ab49 100644 (file)
@@ -229,9 +229,12 @@ void RenderListBox::calcHeight()
     RenderBlock::calcHeight();
     
     if (m_vBar) {
-        m_vBar->setEnabled(numVisibleItems() < numItems());
+        bool enabled = numVisibleItems() < numItems();
+        m_vBar->setEnabled(enabled);
         m_vBar->setSteps(1, min(1, numVisibleItems() - 1), itemHeight);
         m_vBar->setProportion(numVisibleItems(), numItems());
+        if (!enabled)
+            m_indexOffset = 0;
     }
 }