Reviewed by John.

        More menulist cleanup/tweaking.

        * html/HTMLSelectElement.cpp:
        (WebCore::HTMLSelectElement::setSelectedIndex): Renamed 'index'
        parameter to 'optionIndex' for clarity.
        (WebCore::HTMLSelectElement::menuListDefaultEventHandler): Skip over
        items that are not enabled <option> elements when using arrow keys, and
        make sure we pass an option index to setSelectedIndex.
        * platform/PopupMenu.h:
        (WebCore::PopupMenu::setWasClicked): Specify default argument.
        * rendering/RenderMenuList.cpp:
        (WebCore::RenderMenuList::setTextFromOption): Strip whitespace from
        option text before setting the control text so that options within
        <optgroup>s don't appear indented in the actual control.

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

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index f08f6b193ab942a2441c8b07c7736846465d03fb..0bb7469fca8523679acd7cfe1c590f278d03888f 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2006-10-16  Adam Roben  <aroben@apple.com>
+
+        Reviewed by John.
+
+        More menulist cleanup/tweaking.
+
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::setSelectedIndex): Renamed 'index'
+        parameter to 'optionIndex' for clarity.
+        (WebCore::HTMLSelectElement::menuListDefaultEventHandler): Skip over
+        items that are not enabled <option> elements when using arrow keys, and
+        make sure we pass an option index to setSelectedIndex.
+        * platform/PopupMenu.h:
+        (WebCore::PopupMenu::setWasClicked): Specify default argument.
+        * rendering/RenderMenuList.cpp:
+        (WebCore::RenderMenuList::setTextFromOption): Strip whitespace from
+        option text before setting the control text so that options within
+        <optgroup>s don't appear indented in the actual control.
+
 2006-10-16  Sam Weinig  <sam.weinig@gmail.com>
 
         Reviewed by Adam.

diff --git a/WebCore/html/HTMLSelectElement.cpp b/WebCore/html/HTMLSelectElement.cpp
index d4735b628e4133963916cc2d34d938ed5048339d..5ae4252e5d616bc2acc220c4eddd5e8b46fbfdd7 100644 (file)
--- a/WebCore/html/HTMLSelectElement.cpp
+++ b/WebCore/html/HTMLSelectElement.cpp
@@ -153,10 +153,10 @@ void HTMLSelectElement::deselectItems(HTMLOptionElement* excludeElement)
     }
 }
 
-void HTMLSelectElement::setSelectedIndex(int index, bool deselect, bool fireOnChange)
+void HTMLSelectElement::setSelectedIndex(int optionIndex, bool deselect, bool fireOnChange)
 {
     const Vector<HTMLElement*>& items = listItems();
-    int listIndex = optionToListIndex(index);
+    int listIndex = optionToListIndex(optionIndex);
     HTMLOptionElement* element = 0;
     if (listIndex >= 0) {
         element = static_cast<HTMLOptionElement*>(items[listIndex]);
@@ -165,7 +165,7 @@ void HTMLSelectElement::setSelectedIndex(int index, bool deselect, bool fireOnCh
     if (deselect)
         deselectItems(element);
     if (fireOnChange) {
-        m_lastOnChangeIndex = index;
+        m_lastOnChangeIndex = optionIndex;
         onChange();
     }
 }
@@ -560,17 +560,27 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event* evt)
             handled = true;
         }
 #else
-        int index = optionToListIndex(selectedIndex());
+        int listIndex = optionToListIndex(selectedIndex());
         if (keyIdentifier == "Down" || keyIdentifier == "Right") {
-            if (index < listItems().size() - 1)
-                setSelectedIndex(++index);
+            size_t size = listItems().size();
+            for (listIndex += 1;
+                 listIndex >= 0 && listIndex < size && (listItems()[listIndex]->disabled() || !listItems()[listIndex]->hasTagName(optionTag));
+                 ++listIndex);
+            
+            if (listIndex >= 0 && listIndex < size)
+                setSelectedIndex(listToOptionIndex(listIndex));
             handled = true;
         } else if (keyIdentifier == "Up" || keyIdentifier == "Left") {
-            if (index > 0)
-                setSelectedIndex(--index);
+            size_t size = listItems().size();
+            for (listIndex -= 1;
+                 listIndex >= 0 && listIndex < size && (listItems()[listIndex]->disabled() || !listItems()[listIndex]->hasTagName(optionTag));
+                 --listIndex);
+            
+            if (listIndex >= 0 && listIndex < size)
+                setSelectedIndex(listToOptionIndex(listIndex));
             handled = true;
-        } else if (keyIdentifier == "Enter" && index != m_lastOnChangeIndex) {
-            setSelectedIndex(index, true, true);
+        } else if (keyIdentifier == "Enter" && listIndex != m_lastOnChangeIndex) {
+            setSelectedIndex(listToOptionIndex(listIndex), true, true);
         }
 
 #endif

diff --git a/WebCore/platform/PopupMenu.h b/WebCore/platform/PopupMenu.h
index 1dc88d1e12df717aed4b34b2082f068b7f4a6cd5..57ca123f3999e7afea63c14614d031142aff512b 100644 (file)
--- a/WebCore/platform/PopupMenu.h
+++ b/WebCore/platform/PopupMenu.h
@@ -32,6 +32,7 @@ class NSPopUpButtonCell;
 #endif
 #elif PLATFORM(WIN)
 typedef struct HWND__* HWND;
+typedef struct tagDRAWITEMSTRUCT* LPDRAWITEMSTRUCT;
 #endif
 
 namespace WebCore {
@@ -58,7 +59,7 @@ public:
     bool down();
     
     bool wasClicked() const { return m_wasClicked; }
-    void setWasClicked(bool b) { m_wasClicked = b; }
+    void setWasClicked(bool b = true) { m_wasClicked = b; }
 
     int focusedIndex() const;
     
@@ -66,6 +67,7 @@ public:
 
 #if PLATFORM(WIN)
     HWND popupHandle() const { return m_popup; }
+    void drawItem(LPDRAWITEMSTRUCT);
 #endif
 
 protected:

diff --git a/WebCore/rendering/RenderMenuList.cpp b/WebCore/rendering/RenderMenuList.cpp
index 42a3cfb52e809100f9b63d9f5c077a1b5a04849a..19d3f4dadf03dcee2ea63b62ffcfcfc691ecda07 100644 (file)
--- a/WebCore/rendering/RenderMenuList.cpp
+++ b/WebCore/rendering/RenderMenuList.cpp
@@ -136,7 +136,7 @@ void RenderMenuList::setTextFromOption(int optionIndex)
         if (element->hasTagName(optionTag))
             text = static_cast<HTMLOptionElement*>(listItems[i])->optionText();
     }
-    setText(text);
+    setText(text.stripWhiteSpace());
 }
 
 void RenderMenuList::setText(const String& s)