Changeset 19008 in webkit

Timestamp:

Jan 21, 2007 8:20:07 AM (17 years ago)

Author:

darin

Message:

Reviewed by Adam and Mitz.

• fix ​http://bugs.webkit.org/show_bug.cgi?id=12347 REGRESSION: list box scrolling broken (fast/forms/listbox-selection.html)

Covered by existing test: fast/forms/listbox-selection.html

• rendering/RenderListBox.h: Added override of layout(), made selectionChanged() no longer an inline. Added private scrollToRevealSelection(), m_scrollToRevealSelectionAfterLayout, and m_inAutoscroll.
• rendering/RenderListBox.cpp: (WebCore::RenderListBox::RenderListBox): Initialize new data members. (WebCore::RenderListBox::updateFromElement): Remove scrolling code. (WebCore::RenderListBox::selectionChanged): Moved from header. Calls repaint and then scrollToRevealSelection, but if we need layout, instead schedules scrollToRevealSelection to be done after layout. (WebCore::RenderListBox::layout): Added. After calling base class, calls scrollToRevealSelection if the m_scrollToRevealSelectionAfterLayout is set. (WebCore::RenderListBox::scrollToRevealSelection): Added. Code was originally in updateFromElement. (WebCore::RenderListBox::autoscroll): Set m_inAutoscroll so that the selectionChanged function knows not to scroll. Also removed the repaint() here because updateListBoxSelection() takes care of repainting by calling selectionChanged().

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderListBox.cpp (modified) (5 diffs)
• rendering/RenderListBox.h (modified) (2 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-01-21  Darin Adler  <darin@apple.com>
+
+        Reviewed by Adam and Mitz.
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=12347
+          REGRESSION: list box scrolling broken (fast/forms/listbox-selection.html)
+
+        Covered by existing test: fast/forms/listbox-selection.html
+
+        * rendering/RenderListBox.h: Added override of layout(), made selectionChanged()
+        no longer an inline. Added private scrollToRevealSelection(),
+        m_scrollToRevealSelectionAfterLayout, and m_inAutoscroll.
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::RenderListBox): Initialize new data members.
+        (WebCore::RenderListBox::updateFromElement): Remove scrolling code.
+        (WebCore::RenderListBox::selectionChanged): Moved from header. Calls repaint
+        and then scrollToRevealSelection, but if we need layout, instead schedules
+        scrollToRevealSelection to be done after layout.
+        (WebCore::RenderListBox::layout): Added. After calling base class, calls
+        scrollToRevealSelection if the m_scrollToRevealSelectionAfterLayout is set.
+        (WebCore::RenderListBox::scrollToRevealSelection): Added. Code was originally
+        in updateFromElement.
+        (WebCore::RenderListBox::autoscroll): Set m_inAutoscroll so that the
+        selectionChanged function knows not to scroll. Also removed the repaint()
+        here because updateListBoxSelection() takes care of repainting by calling
+        selectionChanged().
+
 2007-01-21  David Kilzer  <ddkilzer@webkit.org>
-
-        Reviewed by NOBODY (no-svg build fix).
 
         * page/EventHandler.cpp:

trunk/WebCore/rendering/RenderListBox.cpp

@@ -70 +70 @@
     : RenderBlock(element)
     , m_optionsChanged(true)
+    , m_scrollToRevealSelectionAfterLayout(false)
+    , m_inAutoscroll(false)
     , m_optionsWidth(0)
     , m_indexOffset(0)
@@ -90 +92 @@
 void RenderListBox::updateFromElement()
 {
-    HTMLSelectElement* select = static_cast<HTMLSelectElement*>(node());
-
     if (m_optionsChanged) {
-        const Vector<HTMLElement*>& listItems = select->listItems();
+        const Vector<HTMLElement*>& listItems = static_cast<HTMLSelectElement*>(node())->listItems();
         int size = numItems();
         
@@ -121 +121 @@
         setNeedsLayoutAndMinMaxRecalc();
     }
-    
+}
+
+void RenderListBox::selectionChanged()
+{
+    repaint();
+    if (!m_inAutoscroll) {
+        if (needsLayout())
+            m_scrollToRevealSelectionAfterLayout = true;
+        else
+            scrollToRevealSelection();
+    }
+}
+
+void RenderListBox::layout()
+{
+    RenderBlock::layout();
+    if (m_scrollToRevealSelectionAfterLayout)
+        scrollToRevealSelection();
+}
+
+void RenderListBox::scrollToRevealSelection()
+{    
+    HTMLSelectElement* select = static_cast<HTMLSelectElement*>(node());
+
+    m_scrollToRevealSelectionAfterLayout = false;
+
     int firstIndex = select->optionToListIndex(select->selectedIndex());
-    int lastIndex = select->lastSelectedListIndex();
-    if (firstIndex >= 0 && !listIndexIsVisible(firstIndex) && !listIndexIsVisible(lastIndex))
+    if (firstIndex >= 0 && !listIndexIsVisible(select->lastSelectedListIndex()))
         scrollToRevealElementAtListIndex(firstIndex);
 }
@@ -378 +402 @@
         endIndex = listIndexAtOffset(offsetX, offsetY);
 
-    HTMLSelectElement* select = static_cast<HTMLSelectElement*>(node());
-    if (endIndex >= 0 && select) {
+    if (endIndex >= 0) {
+        HTMLSelectElement* select = static_cast<HTMLSelectElement*>(node());
+        m_inAutoscroll = true;
         if (!select->multiple())
             select->setActiveSelectionAnchorIndex(endIndex);
         select->setActiveSelectionEndIndex(endIndex);
         select->updateListBoxSelection(!select->multiple());
-        repaint();
+        m_inAutoscroll = false;
     }
 }
@@ -394 +419 @@
 
 bool RenderListBox::scrollToRevealElementAtListIndex(int index)
-{    
+{
     if (index < 0 || index >= numItems() || listIndexIsVisible(index))
         return false;

trunk/WebCore/rendering/RenderListBox.h

@@ -65 +65 @@
     virtual void calcHeight();
 
-    void selectionChanged() { repaint(); }
+    virtual void layout();
+
+    void selectionChanged();
 
     void setOptionsChanged(bool changed) { m_optionsChanged = changed; }
@@ -101 +103 @@
     void paintItemBackground(PaintInfo&, int tx, int ty, int listIndex);
     bool listIndexIsVisible(int index);
+    void scrollToRevealSelection();
 
     bool m_optionsChanged;
+    bool m_scrollToRevealSelectionAfterLayout;
+    bool m_inAutoscroll;
     int m_optionsWidth;
     int m_indexOffset;