Reviewed by Adele.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Jul 2006 02:55:47 +0000 (02:55 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Jul 2006 02:55:47 +0000 (02:55 +0000)
        - fix http://bugzilla.opendarwin.org/show_bug.cgi?id=9809
          <rdar://problem/4619515>
          focus ring fails to appear on select element after choosing item from popup

        - includes http://bugzilla.opendarwin.org/show_bug.cgi?id=9853
          improvements to select element, including some storage leak fixes

        * html/HTMLOptionElement.cpp: (WebCore::HTMLOptionElement::index): Use a const
        reference for the list items, so we don't have to copy a vector.
        * html/HTMLSelectElement.cpp:
        (WebCore::HTMLSelectElement::selectedIndex): Ditto.
        (WebCore::HTMLSelectElement::setSelectedIndex): Ditto.
        (WebCore::HTMLSelectElement::length): Ditto.
        (WebCore::HTMLSelectElement::remove): Ditto.
        (WebCore::HTMLSelectElement::value): Ditto.
        (WebCore::HTMLSelectElement::setValue): Ditto.
        (WebCore::HTMLSelectElement::stateValue): Ditto.
        (WebCore::HTMLSelectElement::restoreState): Ditto.
        (WebCore::HTMLSelectElement::appendFormData): Ditto.
        (WebCore::HTMLSelectElement::optionToListIndex): Ditto.
        (WebCore::HTMLSelectElement::listToOptionIndex): Ditto.
        (WebCore::HTMLSelectElement::recalcListItems): Made const, with the appropriate
        fields mutable.
        (WebCore::HTMLSelectElement::reset): Use a const reference for
        the list items, so we don't have to copy the vector. Remove the call to
        setSelectionChanged for the RenderMenuList case.
        (WebCore::HTMLSelectElement::notifyOptionSelected): Ditto, on both counts.
        (WebCore::HTMLSelectElement::defaultEventHandler): Call focus() before showing
        the pop-up.
        * html/HTMLSelectElement.h: The RenderMenuList class is no longer a friend.
        Changed the listItems function to return a const reference to the vector so
        it no longer copies the vector. Removed the const_cast to the call to
        recalcListItems and changed it to a const member function. Made m_recalcListItems
        mutable.
        * rendering/DeprecatedRenderSelect.cpp:
        (WebCore::DeprecatedRenderSelect::updateFromElement): Removed an unnecessary call
        to recalcListItems, which is called automatically. Use a const reference for the
        list items so we don't have to copy a vector.
        (WebCore::DeprecatedRenderSelect::layout): Ditto.
        (WebCore::DeprecatedRenderSelect::selectionChanged): Ditto.
        (WebCore::DeprecatedRenderSelect::updateSelection): Ditto.
        * rendering/RenderMenuList.cpp:
        (WebCore::RenderMenuList::RenderMenuList): Updated for renamed data members.
        (WebCore::RenderMenuList::createInnerBlock): Ditto.
        (WebCore::RenderMenuList::addChild): Ditto.
        (WebCore::RenderMenuList::removeChild): Ditto.
        (WebCore::RenderMenuList::setStyle): Ditto. Also removed code to set the style
        on the pop-up menu, because it's created with the correct style and destroyed
        before it a style change could occur.
        (WebCore::RenderMenuList::updateFromElement): Rearranged code to compute the
        maximum width in a simpler fashion, and to not bother trying to maintain
        the "selected" flags on the elements, since the HTMLSelectElement class
        takes care of that. Store the width as an int. Call setText to set the text
        based on the selected element's option text.
        (WebCore::RenderMenuList::paintObject): Don't check m_inner when setting
        up the clip -- always set up the clip.
        (WebCore::RenderMenuList::calcMinMaxWidth): Use m_optionsWidth directly
        instead of calling ceilf on m_longestWidth.
        (WebCore::RenderMenuList::showPopup): Don't use m_popupMenu to store the
        menu -- instead keep the pointer in a local variable. Get the selected
        index from the HTMLSelectElement.
        (WebCore::RenderMenuList::valueChanged): Call HTMLSelectElement::setSelectedIndex
        to do most of the work.
        * rendering/RenderMenuList.h: Renamed m_inner to m_innerBlock. Removed
        m_popupMenu, m_size, m_selectionChanged, and m_selectedIndex. Renamed
        m_longestWidth to m_optionsWidth and changed it to be an int. Removed
        unneeded override of removeLeftoverAnonymousBoxes function. Removed
        unneeded selectionChanged, setSelectionChanged, updateSelection, and
        hasPopupMenu functions. Removed extra includes.
        * rendering/RenderPopupMenu.cpp: (WebCore::RenderPopupMenu::populate):
        Change to iterate the list items instead of iterating all children
        of the select node.
        * rendering/RenderPopupMenu.h: Renamed getRenderMenuList to menuList.
        * rendering/RenderPopupMenuMac.mm:
        (WebCore::RenderPopupMenuMac::populate): Moved code to clear and create
        the pop-up here from the caller. Removed an extra retain that would cause
        the NSPopUpButtonCell to leak.
        (WebCore::RenderPopupMenuMac::showPopup): Removed unnecessary code to
        create the pop-up, which is now in populate, and also the call to the
        clear function, for the same reason. Reorganized code to make it a bit
        more readable. Removed an unnecessary if to check if frame is nil.
        Used a RefPtr to make sure we don't make a call on a frame after it's
        deleted. As part of the reorganization fixed a problem where we'd retain
        the event and then return early without releasing it in one case.
        (WebCore::RenderPopupMenuMac::addSeparator): Tweaked a little.
        (WebCore::RenderPopupMenuMac::addGroupLabel): Grouped all the code to
        manage the NSMenu at the bottom of the function.
        (WebCore::RenderPopupMenuMac::addOption): Ditto.

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

WebCore/ChangeLog
WebCore/html/HTMLOptionElement.cpp
WebCore/html/HTMLSelectElement.cpp
WebCore/html/HTMLSelectElement.h
WebCore/rendering/DeprecatedRenderSelect.cpp
WebCore/rendering/RenderMenuList.cpp
WebCore/rendering/RenderMenuList.h
WebCore/rendering/RenderPopupMenu.cpp
WebCore/rendering/RenderPopupMenu.h
WebCore/rendering/RenderPopupMenuMac.mm

index 34a646aad0969f42f1ec4cbe06bb4dceb00325dd..434959dbab2bc3c6af257f824ddea1f0793e7a74 100644 (file)
@@ -1,3 +1,96 @@
+2006-07-11  Darin Adler  <darin@apple.com>
+
+        Reviewed by Adele.
+
+        - fix http://bugzilla.opendarwin.org/show_bug.cgi?id=9809
+          <rdar://problem/4619515>
+          focus ring fails to appear on select element after choosing item from popup
+
+        - includes http://bugzilla.opendarwin.org/show_bug.cgi?id=9853
+          improvements to select element, including some storage leak fixes
+
+        * html/HTMLOptionElement.cpp: (WebCore::HTMLOptionElement::index): Use a const
+        reference for the list items, so we don't have to copy a vector.
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::selectedIndex): Ditto.
+        (WebCore::HTMLSelectElement::setSelectedIndex): Ditto.
+        (WebCore::HTMLSelectElement::length): Ditto.
+        (WebCore::HTMLSelectElement::remove): Ditto.
+        (WebCore::HTMLSelectElement::value): Ditto.
+        (WebCore::HTMLSelectElement::setValue): Ditto.
+        (WebCore::HTMLSelectElement::stateValue): Ditto.
+        (WebCore::HTMLSelectElement::restoreState): Ditto.
+        (WebCore::HTMLSelectElement::appendFormData): Ditto.
+        (WebCore::HTMLSelectElement::optionToListIndex): Ditto.
+        (WebCore::HTMLSelectElement::listToOptionIndex): Ditto.
+        (WebCore::HTMLSelectElement::recalcListItems): Made const, with the appropriate
+        fields mutable.
+        (WebCore::HTMLSelectElement::reset): Use a const reference for
+        the list items, so we don't have to copy the vector. Remove the call to
+        setSelectionChanged for the RenderMenuList case.
+        (WebCore::HTMLSelectElement::notifyOptionSelected): Ditto, on both counts.
+        (WebCore::HTMLSelectElement::defaultEventHandler): Call focus() before showing
+        the pop-up.
+        * html/HTMLSelectElement.h: The RenderMenuList class is no longer a friend.
+        Changed the listItems function to return a const reference to the vector so
+        it no longer copies the vector. Removed the const_cast to the call to
+        recalcListItems and changed it to a const member function. Made m_recalcListItems
+        mutable.
+        * rendering/DeprecatedRenderSelect.cpp:
+        (WebCore::DeprecatedRenderSelect::updateFromElement): Removed an unnecessary call
+        to recalcListItems, which is called automatically. Use a const reference for the
+        list items so we don't have to copy a vector.
+        (WebCore::DeprecatedRenderSelect::layout): Ditto.
+        (WebCore::DeprecatedRenderSelect::selectionChanged): Ditto.
+        (WebCore::DeprecatedRenderSelect::updateSelection): Ditto.
+        * rendering/RenderMenuList.cpp:
+        (WebCore::RenderMenuList::RenderMenuList): Updated for renamed data members.
+        (WebCore::RenderMenuList::createInnerBlock): Ditto.
+        (WebCore::RenderMenuList::addChild): Ditto.
+        (WebCore::RenderMenuList::removeChild): Ditto.
+        (WebCore::RenderMenuList::setStyle): Ditto. Also removed code to set the style
+        on the pop-up menu, because it's created with the correct style and destroyed
+        before it a style change could occur.
+        (WebCore::RenderMenuList::updateFromElement): Rearranged code to compute the
+        maximum width in a simpler fashion, and to not bother trying to maintain
+        the "selected" flags on the elements, since the HTMLSelectElement class
+        takes care of that. Store the width as an int. Call setText to set the text
+        based on the selected element's option text.
+        (WebCore::RenderMenuList::paintObject): Don't check m_inner when setting
+        up the clip -- always set up the clip.
+        (WebCore::RenderMenuList::calcMinMaxWidth): Use m_optionsWidth directly
+        instead of calling ceilf on m_longestWidth.
+        (WebCore::RenderMenuList::showPopup): Don't use m_popupMenu to store the
+        menu -- instead keep the pointer in a local variable. Get the selected
+        index from the HTMLSelectElement.
+        (WebCore::RenderMenuList::valueChanged): Call HTMLSelectElement::setSelectedIndex
+        to do most of the work.
+        * rendering/RenderMenuList.h: Renamed m_inner to m_innerBlock. Removed
+        m_popupMenu, m_size, m_selectionChanged, and m_selectedIndex. Renamed
+        m_longestWidth to m_optionsWidth and changed it to be an int. Removed
+        unneeded override of removeLeftoverAnonymousBoxes function. Removed
+        unneeded selectionChanged, setSelectionChanged, updateSelection, and
+        hasPopupMenu functions. Removed extra includes.
+        * rendering/RenderPopupMenu.cpp: (WebCore::RenderPopupMenu::populate):
+        Change to iterate the list items instead of iterating all children
+        of the select node.
+        * rendering/RenderPopupMenu.h: Renamed getRenderMenuList to menuList.
+        * rendering/RenderPopupMenuMac.mm:
+        (WebCore::RenderPopupMenuMac::populate): Moved code to clear and create
+        the pop-up here from the caller. Removed an extra retain that would cause
+        the NSPopUpButtonCell to leak.
+        (WebCore::RenderPopupMenuMac::showPopup): Removed unnecessary code to
+        create the pop-up, which is now in populate, and also the call to the
+        clear function, for the same reason. Reorganized code to make it a bit
+        more readable. Removed an unnecessary if to check if frame is nil.
+        Used a RefPtr to make sure we don't make a call on a frame after it's
+        deleted. As part of the reorganization fixed a problem where we'd retain
+        the event and then return early without releasing it in one case.
+        (WebCore::RenderPopupMenuMac::addSeparator): Tweaked a little.
+        (WebCore::RenderPopupMenuMac::addGroupLabel): Grouped all the code to
+        manage the NSMenu at the bottom of the function.
+        (WebCore::RenderPopupMenuMac::addOption): Ditto.
+
 2006-07-11  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by levi
index 7aaf6cd132674590eda957665bc5e14f5cd36551..ff1c1b535f91de07ba45cd6b11ed6097bbf797e5 100644 (file)
@@ -124,7 +124,7 @@ int HTMLOptionElement::index() const
     // we won't forget to update a member variable in some cases...
     HTMLSelectElement *select = getSelect();
     if (select) {
-        Vector<HTMLElement*> items = select->listItems();
+        const Vector<HTMLElement*>& items = select->listItems();
         int l = items.size();
         int optionIndex = 0;
         for(int i = 0; i < l; i++) {
@@ -162,19 +162,18 @@ String HTMLOptionElement::value() const
     return text().deprecatedString().stripWhiteSpace();
 }
 
-void HTMLOptionElement::setValue(const String &value)
+void HTMLOptionElement::setValue(const Stringvalue)
 {
     setAttribute(valueAttr, value);
 }
 
-void HTMLOptionElement::setSelected(bool _selected)
+void HTMLOptionElement::setSelected(bool selected)
 {
-    if(m_selected == _selected)
+    if (m_selected == selected)
         return;
-    m_selected = _selected;
-    HTMLSelectElement *select = getSelect();
-    if (select)
-        select->notifyOptionSelected(this,_selected);
+    m_selected = selected;
+    if (HTMLSelectElement* select = getSelect())
+        select->notifyOptionSelected(this, selected);
 }
 
 void HTMLOptionElement::childrenChanged()
index 3706cae6b0493d1bf3123aabc4b71e10d490cf47..58d592c654fccd439e80ed69d71e9ea342f4ab93 100644 (file)
@@ -98,7 +98,7 @@ int HTMLSelectElement::selectedIndex() const
 {
     // return the number of the first option selected
     unsigned o = 0;
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
     for (unsigned int i = 0; i < items.size(); i++) {
         if (items[i]->hasLocalName(optionTag)) {
             if (static_cast<HTMLOptionElement*>(items[i])->selected())
@@ -109,10 +109,10 @@ int HTMLSelectElement::selectedIndex() const
     return -1;
 }
 
-void HTMLSelectElement::setSelectedIndex( int index, bool deselect )
+void HTMLSelectElement::setSelectedIndex(int index, bool deselect)
 {
     // deselect all other options and select only the new one
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
     int listIndex;
     if (deselect) {
         for (listIndex = 0; listIndex < int(items.size()); listIndex++) {
@@ -131,7 +131,7 @@ int HTMLSelectElement::length() const
 {
     int len = 0;
     unsigned i;
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
     for (i = 0; i < items.size(); i++) {
         if (items[i]->hasLocalName(optionTag))
             len++;
@@ -156,7 +156,7 @@ void HTMLSelectElement::remove(int index)
     ExceptionCode ec = 0;
     int listIndex = optionToListIndex(index);
 
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
     if (listIndex < 0 || index >= int(items.size()))
         return; // ### what should we do ? remove the last item?
 
@@ -168,7 +168,7 @@ void HTMLSelectElement::remove(int index)
 String HTMLSelectElement::value()
 {
     unsigned i;
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
     for (i = 0; i < items.size(); i++) {
         if (items[i]->hasLocalName(optionTag) && static_cast<HTMLOptionElement*>(items[i])->selected())
             return static_cast<HTMLOptionElement*>(items[i])->value();
@@ -182,7 +182,7 @@ void HTMLSelectElement::setValue(const String &value)
         return;
     // find the option with value() matching the given parameter
     // and make it the current selection.
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
     for (unsigned i = 0; i < items.size(); i++)
         if (items[i]->hasLocalName(optionTag) && static_cast<HTMLOptionElement*>(items[i])->value() == value) {
             static_cast<HTMLOptionElement*>(items[i])->setSelected(true);
@@ -192,7 +192,7 @@ void HTMLSelectElement::setValue(const String &value)
 
 String HTMLSelectElement::stateValue() const
 {
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
     int l = items.size();
     Vector<char, 1024> characters(l);
     for (int i = 0; i < l; ++i) {
@@ -207,7 +207,7 @@ void HTMLSelectElement::restoreState(const String& state)
 {
     recalcListItems();
 
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
     int l = items.size();
     for (int i = 0; i < l; i++)
         if (items[i]->hasLocalName(optionTag))
@@ -308,7 +308,7 @@ RenderObject *HTMLSelectElement::createRenderer(RenderArena *arena, RenderStyle
 bool HTMLSelectElement::appendFormData(FormDataList& list, bool)
 {
     bool successful = false;
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
 
     unsigned i;
     for (i = 0; i < items.size(); i++) {
@@ -339,7 +339,7 @@ bool HTMLSelectElement::appendFormData(FormDataList& list, bool)
 
 int HTMLSelectElement::optionToListIndex(int optionIndex) const
 {
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
     if (optionIndex < 0 || optionIndex >= int(items.size()))
         return -1;
 
@@ -357,14 +357,13 @@ int HTMLSelectElement::optionToListIndex(int optionIndex) const
 
 int HTMLSelectElement::listToOptionIndex(int listIndex) const
 {
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
     if (listIndex < 0 || listIndex >= int(items.size()) ||
         !items[listIndex]->hasLocalName(optionTag))
         return -1;
 
     int optionIndex = 0; // actual index of option not counting OPTGROUP entries that may be in list
-    int i;
-    for (i = 0; i < listIndex; i++)
+    for (int i = 0; i < listIndex; i++)
         if (items[i]->hasLocalName(optionTag))
             optionIndex++;
     return optionIndex;
@@ -375,7 +374,7 @@ PassRefPtr<HTMLOptionsCollection> HTMLSelectElement::options()
     return new HTMLOptionsCollection(this);
 }
 
-void HTMLSelectElement::recalcListItems()
+void HTMLSelectElement::recalcListItems() const
 {
     Node* current = firstChild();
     m_listItems.clear();
@@ -432,7 +431,7 @@ void HTMLSelectElement::reset()
 {
     bool optionSelected = false;
     HTMLOptionElement* firstOption = 0;
-    Vector<HTMLElement*> items = listItems();
+    const Vector<HTMLElement*>& items = listItems();
     unsigned i;
     for (i = 0; i < items.size(); i++) {
         if (items[i]->hasLocalName(optionTag)) {
@@ -448,12 +447,8 @@ void HTMLSelectElement::reset()
     }
     if (!optionSelected && firstOption)
         firstOption->setSelected(true);
-    if (renderer()) {
-        if (shouldUseMenuList())
-            static_cast<RenderMenuList*>(renderer())->setSelectionChanged(true);
-        else
-            static_cast<DeprecatedRenderSelect*>(renderer())->setSelectionChanged(true);
-    }
+    if (renderer() && !shouldUseMenuList())
+        static_cast<DeprecatedRenderSelect*>(renderer())->setSelectionChanged(true);
     setChanged(true);
 }
 
@@ -461,19 +456,15 @@ void HTMLSelectElement::notifyOptionSelected(HTMLOptionElement *selectedOption,
 {
     if (selected && !m_multiple) {
         // deselect all other options
-        Vector<HTMLElement*> items = listItems();
+        const Vector<HTMLElement*>& items = listItems();
         unsigned i;
         for (i = 0; i < items.size(); i++) {
             if (items[i]->hasLocalName(optionTag))
                 static_cast<HTMLOptionElement*>(items[i])->m_selected = (items[i] == selectedOption);
         }
     }
-    if (renderer()) {
-        if (shouldUseMenuList())
-            static_cast<RenderMenuList*>(renderer())->setSelectionChanged(true);
-        else
-            static_cast<DeprecatedRenderSelect*>(renderer())->setSelectionChanged(true);
-    }
+    if (renderer() && !shouldUseMenuList())
+        static_cast<DeprecatedRenderSelect*>(renderer())->setSelectionChanged(true);
 
     setChanged(true);
 }
@@ -491,11 +482,13 @@ void HTMLSelectElement::defaultEventHandler(Event *evt)
             evt->setDefaultHandled();
         }
         if ((keyIdentifier == "Down" || keyIdentifier == "Up" || keyIdentifier == "U+000020") && renderer() && shouldUseMenuList()) {
+            focus();
             static_cast<RenderMenuList*>(renderer())->showPopup();
             evt->setDefaultHandled();
         }
     }
     if (evt->type() == mousedownEvent && renderer() && shouldUseMenuList()) {
+        focus();
         static_cast<RenderMenuList*>(renderer())->showPopup();
         evt->setDefaultHandled();
     }
index 14e0f98e06a75e902801dc9cd8a82759defdcb13..375ba11660393cff74d00be54e5e80ed6458f0ec 100644 (file)
 
 namespace WebCore {
 
+class DeprecatedRenderSelect;
 class HTMLOptionElement;
 class HTMLOptionsCollection;
-class DeprecatedRenderSelect;
 
 class HTMLSelectElement : public HTMLGenericFormElement {
     friend class DeprecatedRenderSelect;
-    friend class RenderMenuList;
-
 public:
     HTMLSelectElement(Document*, HTMLFormElement* = 0);
     HTMLSelectElement(const QualifiedName& tagName, Document*, HTMLFormElement* = 0);
@@ -100,10 +98,10 @@ public:
 
     void setRecalcListItems();
 
-    Vector<HTMLElement*> listItems() const
+    const Vector<HTMLElement*>& listItems() const
     {
         if (m_recalcListItems)
-            const_cast<HTMLSelectElement*>(this)->recalcListItems();
+            recalcListItems();
         return m_listItems;
     }
     virtual void reset();
@@ -119,23 +117,23 @@ public:
     void setOption(unsigned index, HTMLOptionElement*, ExceptionCode&);
     void setLength(unsigned, ExceptionCode&);
 
-    virtual Node* namedItem(const String &name, bool caseSensitive = true);
+    virtual Node* namedItem(const Stringname, bool caseSensitive = true);
 
     HTMLCollection::CollectionInfo* collectionInfo() { return &m_collectionInfo; }
 
 private:
-    void recalcListItems();
+    void recalcListItems() const;
     bool shouldUseMenuList() const { return !m_multiple && m_size <= 1; }
 
     mutable Vector<HTMLElement*> m_listItems;
     int m_minwidth;
     int m_size;
     bool m_multiple;
-    bool m_recalcListItems;
-    
+    mutable bool m_recalcListItems;
+
     HTMLCollection::CollectionInfo m_collectionInfo;
 };
 
-} //namespace
+} // namespace
 
 #endif
index 6df1c18f4c1ce69a4b7c56d56548068a66c975d3..bcb9bb6ec116cd88382c29b69a9cec3e906b82f5 100644 (file)
@@ -77,9 +77,8 @@ void DeprecatedRenderSelect::updateFromElement()
 
     // update contents listbox/combobox based on options in m_element
     if (m_optionsChanged) {
-        if (static_cast<HTMLSelectElement*>(node())->m_recalcListItems)
-            static_cast<HTMLSelectElement*>(node())->recalcListItems();
-        Vector<HTMLElement*> listItems = static_cast<HTMLSelectElement*>(node())->listItems();
+        static_cast<HTMLSelectElement*>(node())->recalcListItems();
+        const Vector<HTMLElement*>& listItems = static_cast<HTMLSelectElement*>(node())->listItems();
         int listIndex;
 
         static_cast<ListBox*>(m_widget)->clear();
@@ -189,7 +188,7 @@ void DeprecatedRenderSelect::layout()
     RenderFormElement::layout();
 
     // and now disable the widget in case there is no <option> given
-    Vector<HTMLElement*> listItems = static_cast<HTMLSelectElement*>(node())->listItems();
+    const Vector<HTMLElement*>& listItems = static_cast<HTMLSelectElement*>(node())->listItems();
 
     bool foundOption = false;
     for (unsigned i = 0; i < listItems.size() && !foundOption; i++)
@@ -205,7 +204,7 @@ void DeprecatedRenderSelect::selectionChanged(Widget*)
 
     // don't use listItems() here as we have to avoid recalculations - changing the
     // option list will make use update options not in the way the user expects them
-    Vector<HTMLElement*> listItems = static_cast<HTMLSelectElement*>(node())->m_listItems;
+    const Vector<HTMLElement*>& listItems = static_cast<HTMLSelectElement*>(node())->m_listItems;
     int j = 0;
     unsigned size = listItems.size();
     for (unsigned i = 0; i < size; i++) {
@@ -235,7 +234,7 @@ ListBox* DeprecatedRenderSelect::createListBox()
 
 void DeprecatedRenderSelect::updateSelection()
 {
-    Vector<HTMLElement*> listItems = static_cast<HTMLSelectElement*>(node())->listItems();
+    const Vector<HTMLElement*>& listItems = static_cast<HTMLSelectElement*>(node())->listItems();
     int i;
     // if multi-select, we select only the new selected index
     ListBox *listBox = static_cast<ListBox*>(m_widget);
index 94285a94b6618f145728e284da91fc4927571a6a..e2ed427de56103ead8000e84ced1b259adbae47b 100644 (file)
 #include "RenderMenuList.h"
 
 #include "Document.h"
-#include "HTMLNames.h"
-#include "HTMLOptionElement.h"
 #include "FrameView.h"
 #include "GraphicsContext.h"
+#include "HTMLNames.h"
+#include "HTMLOptionElement.h"
+#include "HTMLSelectElement.h"
 #include "RenderPopupMenu.h"
 #include "RenderText.h"
 #include "RenderTheme.h"
@@ -42,45 +43,40 @@ using namespace HTMLNames;
 RenderMenuList::RenderMenuList(HTMLSelectElement* element)
     : RenderFlexibleBox(element)
     , m_buttonText(0)
-    , m_inner(0)
-    , m_popupMenu(0)
-    , m_size(element->size())
-    , m_selectionChanged(true)
+    , m_innerBlock(0)
     , m_optionsChanged(true)
-    , m_longestWidth(0)
-    , m_selectedIndex(0)
+    , m_optionsWidth(0)
 {
 }
 
 void RenderMenuList::createInnerBlock()
 {
-    if (m_inner) {
-        ASSERT(firstChild() == m_inner);
-        ASSERT(!m_inner->nextSibling());
+    if (m_innerBlock) {
+        ASSERT(firstChild() == m_innerBlock);
+        ASSERT(!m_innerBlock->nextSibling());
         return;
     }
 
     // Create an anonymous block.
     ASSERT(!firstChild());
-    m_inner = createAnonymousBlock();
-    m_inner->style()->setBoxFlex(1.0f);
-    RenderFlexibleBox::addChild(m_inner);
+    m_innerBlock = createAnonymousBlock();
+    m_innerBlock->style()->setBoxFlex(1.0f);
+    RenderFlexibleBox::addChild(m_innerBlock);
 }
 
 void RenderMenuList::addChild(RenderObject* newChild, RenderObject* beforeChild)
 {
     createInnerBlock();
-    m_inner->addChild(newChild, beforeChild);
+    m_innerBlock->addChild(newChild, beforeChild);
 }
 
 void RenderMenuList::removeChild(RenderObject* oldChild)
 {
-    if (oldChild == m_inner || !m_inner) {
+    if (oldChild == m_innerBlock || !m_innerBlock) {
         RenderFlexibleBox::removeChild(oldChild);
-        m_inner = 0;
-    }
-    else
-        m_inner->removeChild(oldChild);
+        m_innerBlock = 0;
+    } else
+        m_innerBlock->removeChild(oldChild);
 }
 
 void RenderMenuList::setStyle(RenderStyle* style)
@@ -88,46 +84,40 @@ void RenderMenuList::setStyle(RenderStyle* style)
     RenderBlock::setStyle(style);
     if (m_buttonText)
         m_buttonText->setStyle(style);
-    if (m_inner)
-        m_inner->style()->setBoxFlex(1.0f);
-    if (m_popupMenu) {
-        RenderStyle* newStyle = new (renderArena()) RenderStyle;
-        newStyle->inheritFrom(style);
-        m_popupMenu->setStyle(newStyle);
-    }
+    if (m_innerBlock)
+        m_innerBlock->style()->setBoxFlex(1.0f);
     setReplaced(isInline());
 }
 
 void RenderMenuList::updateFromElement()
 {
-    if (m_optionsChanged) {
-        HTMLSelectElement* select = static_cast<HTMLSelectElement*>(node());
-        if (select->m_recalcListItems)
-            select->recalcListItems();
-        
-        Vector<HTMLElement*> listItems = select->listItems();
-        bool found = false;
-        unsigned firstOption = listItems.size();
-        int i = listItems.size();
-        m_longestWidth = 0;
-        while (i--)
-            if (listItems[i]->hasTagName(optionTag)) {
-                HTMLOptionElement* element = static_cast<HTMLOptionElement*>(listItems[i]);
-                String text = element->optionText();
-                float stringWidth = (text.isNull() || text.isEmpty()) ? 0 : style()->font().floatWidth(TextRun(text.impl()), TextStyle(0, 0, 0, false, false, false, false));
-                if (stringWidth > m_longestWidth)
-                    m_longestWidth = stringWidth;
-                if (found)
-                    element->m_selected = false;
-                else if (element->selected()) {
-                    setText(text);
-                    m_selectedIndex = i;
-                    found = true;
-                }
-                firstOption = i;
+    HTMLSelectElement* select = static_cast<HTMLSelectElement*>(node());
+    const Vector<HTMLElement*>& listItems = select->listItems();
+    int size = listItems.size();
+
+    if (m_optionsChanged) {        
+        float width = 0;
+        TextStyle textStyle(0, 0, 0, false, false, false, false);
+        for (int i = 0; i < size; ++i) {
+            HTMLElement* element = listItems[i];
+            if (element->hasTagName(optionTag)) {
+                String text = static_cast<HTMLOptionElement*>(element)->optionText();
+                if (!text.isEmpty())
+                    width = max(width, style()->font().floatWidth(TextRun(text.impl()), textStyle));
             }
+        }
+        m_optionsWidth = static_cast<int>(ceilf(width));
         m_optionsChanged = false;
     }
+
+    int i = select->selectedIndex();
+    String text = "";
+    if (i >= 0 && i < size) {
+        HTMLElement* element = listItems[i];
+        if (element->hasTagName(optionTag))
+            text = static_cast<HTMLOptionElement*>(listItems[i])->optionText();
+    }
+    setText(text);
 }
 
 void RenderMenuList::setText(const String& s)
@@ -148,23 +138,24 @@ void RenderMenuList::setText(const String& s)
     }
 }
 
-void RenderMenuList::paintObject(PaintInfo& i, int _tx, int _ty)
+void RenderMenuList::paintObject(PaintInfo& i, int x, int y)
 {
     // Push a clip.
-    if (m_inner && i.phase == PaintPhaseForeground) {
-        IntRect clipRect(_tx + borderLeft(), _ty + borderTop(),
-            width() - borderLeft() - borderRight(), height() - borderBottom() - borderTop());
-        if (clipRect.width() == 0 || clipRect.height() == 0)
+    if (i.phase == PaintPhaseForeground) {
+        IntRect clipRect(x + borderLeft(), y + borderTop(),
+            width() - borderLeft() - borderRight(),
+            height() - borderBottom() - borderTop());
+        if (clipRect.isEmpty())
             return;
         i.p->save();
         i.p->addClip(clipRect);
     }
-    
+
     // Paint the children.
-    RenderBlock::paintObject(i, _tx, _ty);
-    
+    RenderBlock::paintObject(i, x, y);
+
     // Pop the clip.
-    if (m_inner && i.phase == PaintPhaseForeground)
+    if (i.phase == PaintPhaseForeground)
         i.p->restore();
 }
 
@@ -179,8 +170,8 @@ void RenderMenuList::calcMinMaxWidth()
     if (style()->width().isFixed() && style()->width().value() > 0)
         m_minWidth = m_maxWidth = calcContentBoxWidth(style()->width().value());
     else
-        m_maxWidth = max((int)ceilf(m_longestWidth), theme()->minimumTextSize(style()));
-    
+        m_maxWidth = max(m_optionsWidth, theme()->minimumTextSize(style()));
+
     if (style()->minWidth().isFixed() && style()->minWidth().value() > 0) {
         m_maxWidth = max(m_maxWidth, calcContentBoxWidth(style()->minWidth().value()));
         m_minWidth = max(m_minWidth, calcContentBoxWidth(style()->minWidth().value()));
@@ -188,7 +179,7 @@ void RenderMenuList::calcMinMaxWidth()
         m_minWidth = 0;
     else
         m_minWidth = m_maxWidth;
-    
+
     if (style()->maxWidth().isFixed() && style()->maxWidth().value() != undefinedLength) {
         m_maxWidth = min(m_maxWidth, calcContentBoxWidth(style()->maxWidth().value()));
         m_minWidth = min(m_minWidth, calcContentBoxWidth(style()->maxWidth().value()));
@@ -203,46 +194,25 @@ void RenderMenuList::calcMinMaxWidth()
 
 void RenderMenuList::showPopup()
 {
-    if (!m_popupMenu) {
-        // Create m_inner here so it ends up as the first child.
-        // This is important because otherwise we might try to create m_inner
-        // inside the showPopup call and it would fail.
-        createInnerBlock();
-        m_popupMenu = theme()->createPopupMenu(renderArena(), document());
-        RenderStyle* newStyle = new (renderArena()) RenderStyle;
-        newStyle->inheritFrom(style());
-        m_popupMenu->setStyle(newStyle);
-        RenderFlexibleBox::addChild(m_popupMenu);
-    }
-    m_popupMenu->showPopup(absoluteBoundingBoxRect(), document()->view(), m_selectedIndex);
-    m_popupMenu->destroy();
-    m_popupMenu = 0;
-}
-
-void RenderMenuList::layout()
-{
-    RenderFlexibleBox::layout();
-}
-
-void RenderMenuList::updateSelection()
-{
+    // Create m_innerBlock here so it ends up as the first child.
+    // This is important because otherwise we might try to create m_innerBlock
+    // inside the showPopup call and it would fail.
+    createInnerBlock();
+    RenderPopupMenu* menu = theme()->createPopupMenu(renderArena(), document());
+    RenderStyle* newStyle = new (renderArena()) RenderStyle;
+    newStyle->inheritFrom(style());
+    menu->setStyle(newStyle);
+    RenderFlexibleBox::addChild(menu);
+    menu->showPopup(absoluteBoundingBoxRect(), document()->view(),
+        static_cast<HTMLSelectElement*>(node())->selectedIndex());
+    menu->destroy();
 }
 
 void RenderMenuList::valueChanged(unsigned index)
 {
-    m_selectedIndex = index;
-    Vector<HTMLElement*> listItems = static_cast<HTMLSelectElement*>(node())->listItems();
-
-    for (unsigned i = 0; i < listItems.size(); ++i)
-        if (listItems[i]->hasTagName(optionTag) && i != index)
-            static_cast<HTMLOptionElement*>(listItems[i])->m_selected = false;
-
-    HTMLOptionElement* element = static_cast<HTMLOptionElement*>(listItems[index]);
-    element->m_selected = true;
-    
-    setText(element->optionText());
-    
-    static_cast<HTMLSelectElement*>(node())->onChange();
+    HTMLSelectElement* select = static_cast<HTMLSelectElement*>(node());
+    select->setSelectedIndex(index);
+    select->onChange();
 }
 
 }
index 31367a2e445dcb6bc589e146308a70fb64f97349..921671bd3cc9a6f57989b8b1d7613e8e62c2dcaf 100644 (file)
 #ifndef RenderMenuList_H
 #define RenderMenuList_H
 
-#include "RenderButton.h"
-#include "HTMLSelectElement.h"
+#include "RenderFlexibleBox.h"
 
 namespace WebCore {
 
-class RenderPopupMenu;
+class HTMLSelectElement;
 
-class RenderMenuList : public RenderFlexibleBox
-{
+class RenderMenuList : public RenderFlexibleBox {
 public:
     RenderMenuList(HTMLSelectElement*);
 
     virtual void addChild(RenderObject* newChild, RenderObject *beforeChild = 0);
-    virtual void removeChild(RenderObject* oldChild);
-    virtual void removeLeftoverAnonymousBoxes() {}
+    virtual void removeChild(RenderObject*);
     virtual bool createsAnonymousWrapper() const { return true; }
     virtual bool canHaveChildren() const { return false; }
 
@@ -47,34 +44,23 @@ public:
     virtual void paintObject(PaintInfo&, int tx, int ty);
 
     virtual const char* renderName() const { return "RenderMenuList"; }
-    
-    RenderStyle* createInnerStyle(RenderStyle*);
-    virtual void calcMinMaxWidth();
-    virtual void layout();
 
-    void setOptionsChanged(bool o) { m_optionsChanged = o; }
+    virtual void calcMinMaxWidth();
 
-    bool selectionChanged() { return m_selectionChanged; }
-    void setSelectionChanged(bool selectionChanged) { m_selectionChanged = selectionChanged; }
-    void updateSelection();
-    
     void showPopup();
+
+    void setOptionsChanged(bool c) { m_optionsChanged = c; }
     void valueChanged(unsigned index);
-    bool hasPopupMenu() { return m_popupMenu; }
 
 private:
     void createInnerBlock();
     void setText(const String&);
 
     RenderText* m_buttonText;
-    RenderBlock* m_inner;
-    RenderPopupMenu* m_popupMenu;
+    RenderBlock* m_innerBlock;
 
-    unsigned m_size;
-    bool m_selectionChanged;
     bool m_optionsChanged;
-    float m_longestWidth;
-    int m_selectedIndex;
+    int m_optionsWidth;
 };
 
 }
index 12b381061c557bdca4b9864274471ac18a8e8351..0d58143b44e5521eb57ad590ec24d18ab990651e 100644 (file)
@@ -25,6 +25,7 @@
 #include "HTMLNames.h"
 #include "HTMLOptionElement.h"
 #include "HTMLOptGroupElement.h"
+#include "HTMLSelectElement.h"
 
 namespace WebCore {
 
@@ -37,18 +38,22 @@ RenderPopupMenu::RenderPopupMenu(Node* element)
 
 void RenderPopupMenu::populate()
 {
-    RenderMenuList* select = getRenderMenuList();
-    ASSERT(select);
-    if (!select->node())
+    ASSERT(menuList());
+    HTMLSelectElement* select = static_cast<HTMLSelectElement*>(menuList()->node());
+    if (!select)
         return;
-    //FIXME: Maybe we should just iterate through the select element's list items?
-    for (Node* n = select->node()->firstChild(); n; n = n->traverseNextNode(select->node())) {
-        if (n->hasTagName(optionTag))
-            addOption(static_cast<HTMLOptionElement*>(n));
-        else if (n->hasTagName(optgroupTag))
-            addGroupLabel(static_cast<HTMLOptGroupElement*>(n));
-        else if (n->hasTagName(hrTag))
+    const Vector<HTMLElement*>& items = select->listItems();
+    size_t size = items.size();
+    for (size_t i = 0; i < size; ++i) {
+        HTMLElement* element = items[i];
+        if (element->hasTagName(optionTag))
+            addOption(static_cast<HTMLOptionElement*>(element));
+        else if (element->hasTagName(optgroupTag))
+            addGroupLabel(static_cast<HTMLOptGroupElement*>(element));
+        else if (element->hasTagName(hrTag))
             addSeparator();
+        else
+            ASSERT(0);
     }
 }
 
index d2d9a85c3ad02caa2dae1aa6a456648d7892c287..cfeb2fcac21f97a8774c897b9e1e854e4daa0524 100644 (file)
@@ -34,7 +34,6 @@ class HTMLOptGroupElement;
 class RenderPopupMenu : public RenderBlock {
 public:
     RenderPopupMenu(Node*);
-    virtual ~RenderPopupMenu() {}
     
     virtual const char* renderName() const { return "RenderPopupMenu"; }
 
@@ -42,13 +41,12 @@ public:
     virtual void populate();
     virtual void showPopup(const IntRect&, FrameView*, int index) = 0;
     
-    RenderMenuList* getRenderMenuList() { return static_cast<RenderMenuList*>(parent() ? parent()->parent() : 0); }
+    RenderMenuList* menuList() { return static_cast<RenderMenuList*>(parent() ? parent()->parent() : 0); }
 
 protected:
     virtual void addSeparator() = 0;
     virtual void addGroupLabel(HTMLOptGroupElement*) = 0;
     virtual void addOption(HTMLOptionElement*) = 0;
-
 };
 
 }
index 2c1560bedce26c70d5f58ff9a1811e9f81997ed5..072bb3158dff449c8c953d848caf775a603181a7 100644 (file)
@@ -25,8 +25,8 @@
 #import "FrameMac.h"
 #import "FrameView.h"
 #import "HTMLNames.h"
-#import "HTMLOptionElement.h"
 #import "HTMLOptGroupElement.h"
+#import "HTMLOptionElement.h"
 #import "RenderMenuList.h"
 #import "WebCoreSystemInterface.h"
 
@@ -43,7 +43,7 @@ RenderPopupMenuMac::RenderPopupMenuMac(Node* element)
 RenderPopupMenuMac::~RenderPopupMenuMac()
 {
     if (popup) {
-        [popup setControlView: nil];
+        [popup setControlView:nil];
         [popup release];
     }
 }
@@ -56,95 +56,73 @@ void RenderPopupMenuMac::clear()
 
 void RenderPopupMenuMac::populate()
 {
+    if (popup)
+        [popup removeAllItems];
+    else {
+        popup = [[NSPopUpButtonCell alloc] initTextCell:@"" pullsDown:NO];
+        [popup setUsesItemFromMenu:NO];
+        [popup setAutoenablesItems:NO];
+    }
     BOOL messagesEnabled = [[popup menu] menuChangedMessagesEnabled];
     [[popup menu] setMenuChangedMessagesEnabled:NO];
     RenderPopupMenu::populate();
     [[popup menu] setMenuChangedMessagesEnabled:messagesEnabled];
-
 }
 
 void RenderPopupMenuMac::showPopup(const IntRect& r, FrameView* v, int index)
 {
-    FrameMac* frame = Mac(v->frame());
-    NSEvent* evt = frame->currentEvent();
-    // Save the current event that triggered the popup, so we can clean up our event
-    // state after the NSMenu goes away.
-    [evt retain];
-    
-    if (!popup) {
-        popup = [[[NSPopUpButtonCell alloc] initTextCell:@"" pullsDown:NO] retain];
-        [popup setUsesItemFromMenu:NO];
-        [popup setAutoenablesItems:NO];
-    }
-    // If we decide to cache the NSMenuItems, we shouldn't clear and populate every time we show the popup menu
-    clear();
     populate();
-    
     if ([popup numberOfItems] <= 0)
         return;
-    
     ASSERT([popup numberOfItems] > index);
-    
+
     NSView* view = v->getDocumentView();
+
     [popup attachPopUpWithFrame:r inView:view];
-    
     [popup selectItemAtIndex:index];
     
-    NSPoint location = NSMakePoint(NSMinX(r), NSMaxY(r));
-    float vertOffset;
-    
-    //center it in our title frame
+    NSFont* font = style()->font().getNSFont();
+
     NSRect titleFrame = [popup titleRectForBounds:r];
     if (titleFrame.size.width <= 0 || titleFrame.size.height <= 0)
         titleFrame = r;
-    NSFont* font = style()->font().getNSFont();
-    NSSize sizeOfSelectedItem = titleFrame.size;
-
-    vertOffset = round((NSMaxY(r) - NSMaxY(titleFrame)) + (NSHeight(titleFrame) + sizeOfSelectedItem.height)/2.f);
-
-    //Align us for different fonts
+    float vertOffset = roundf((NSMaxY(r) - NSMaxY(titleFrame)) + NSHeight(titleFrame));
+    // Adjust for fonts other than the system font.
     NSFont* defaultFont = [NSFont systemFontOfSize:[font pointSize]];
     vertOffset += [font descender] - [defaultFont descender];
-
     vertOffset = fmin(NSHeight(r), vertOffset);
 
-    location.x -= 10;
-    location.y -= vertOffset;
-
-//  NSColor* backgroundColor = nsColor(style()->backgroundColor());
     NSMenu* menu = [popup menu];
-    
-    wkPopupMenu(menu, location, floor(NSWidth(r) + 0.5), view, index, font);
-    
-    // update text on button
+    // FIXME: Need to document where this magic number 10 comes from.
+    NSPoint location = NSMakePoint(NSMinX(r) - 10, NSMaxY(r) - vertOffset);
+
+    // Save the current event that triggered the popup, so we can clean up our event
+    // state after the NSMenu goes away.
+    RefPtr<FrameMac> frame = Mac(v->frame());
+    NSEvent* event = [frame->currentEvent() retain];
+
+    wkPopupMenu(menu, location, roundf(NSWidth(r)), view, index, font);
     int newIndex = [popup indexOfSelectedItem];
+    [popup dismissPopUp];
+
     if (index != newIndex && newIndex >= 0)
-        getRenderMenuList()->valueChanged(newIndex);
+        menuList()->valueChanged(newIndex);
 
-    [popup dismissPopUp];
-    
-    if (frame) {
-        // Give WebCore a chance to fix up its event state, since the popup eats all the
-        // events during tracking.
-        frame->sendFakeEventsAfterWidgetTracking(evt);
-    }
-    [evt release];
+    // Give the frame a chance to fix up its event state, since the popup eats all the
+    // events during tracking.
+    frame->sendFakeEventsAfterWidgetTracking(event);
+
+    [event release];
 }
 
 void RenderPopupMenuMac::addSeparator()
 {
-    NSMenuItem *separator = [NSMenuItem separatorItem];
-    [[popup menu] addItem:separator];
+    [[popup menu] addItem:[NSMenuItem separatorItem]];
 }
 
 void RenderPopupMenuMac::addGroupLabel(HTMLOptGroupElement* element)
 {
-    if (!element)
-        return;
     String text = element->groupLabelText();
-    
-    [popup addItemWithTitle:@""];
-    NSMenuItem *menuItem = [popup lastItem];
 
     RenderStyle* s = element->renderStyle();
     if (!s)
@@ -155,27 +133,25 @@ void RenderPopupMenuMac::addGroupLabel(HTMLOptGroupElement* element)
         [attributes setObject:s->font().getNSFont() forKey:NSFontAttributeName];
     if (s->color() != Color::black)
         [attributes setObject:nsColor(s->color()) forKey:NSForegroundColorAttributeName];
-    NSAttributedString *string = [[NSAttributedString alloc] initWithString:text attributes:attributes];
+    NSAttributedString* string = [[NSAttributedString alloc] initWithString:text attributes:attributes];
+    [attributes release];
 
+    [popup addItemWithTitle:@""];
+    NSMenuItem* menuItem = [popup lastItem];
     [menuItem setAttributedTitle:string];
-    [string release];
-    [attributes release];
     [menuItem setEnabled:NO];
+
+    [string release];
 }
 
 void RenderPopupMenuMac::addOption(HTMLOptionElement* element)
 {
-    if (!element)
-        return;
     String text = element->optionText();
     
     bool groupEnabled = true;
     if (element->parentNode() && element->parentNode()->hasTagName(optgroupTag))
         groupEnabled = element->parentNode()->isEnabled();
 
-    [popup addItemWithTitle:@""];
-    NSMenuItem *menuItem = [popup lastItem];
-    
     RenderStyle* s = element->renderStyle();
     if (!s)
         s = style();
@@ -186,13 +162,15 @@ void RenderPopupMenuMac::addOption(HTMLOptionElement* element)
     if (s->color() != Color::black)
         [attributes setObject:nsColor(s->color()) forKey:NSForegroundColorAttributeName];
     // FIXME: Find a way to customize text color when an item is highlighted.
-    NSAttributedString *string = [[NSAttributedString alloc] initWithString:text attributes:attributes];
+    NSAttributedString* string = [[NSAttributedString alloc] initWithString:text attributes:attributes];
+    [attributes release];
 
+    [popup addItemWithTitle:@""];
+    NSMenuItem* menuItem = [popup lastItem];
     [menuItem setAttributedTitle:string];
-    [string release];
-    [attributes release];
     [menuItem setEnabled:groupEnabled && element->isEnabled()];
 
+    [string release];
 }
 
 }