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 89820d357e9d57e088ddc2c85ff21ad5007d8396..55392050d1266fb0e1e5ea475e179f47db3d01e0 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 aa6e7f4e160376bfbaedfa89fedf494484fba442..a8d19284249ab870430cc45733dc8c39415948a0 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 0da11e87c4136fd03eddb3631496335e82f80313..276ab494c94f55c60995b6d1592685f7efc013cc 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;
     }
 }