Reviewed by Eric Seidel.
authorzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Jan 2009 20:40:18 +0000 (20:40 +0000)
committerzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Jan 2009 20:40:18 +0000 (20:40 +0000)
Fixes: https://bugs.webkit.org/show_bug.cgi?id=23465

Further enhancments to share code between HTMLOptionElement and the upcoming WMLOptionElement.

Rename optionText() to textIndentedToRespectGroupLabel() in (HTML)OptionElement, as it fits better.
optionText() returns the options text prefixed with some spaces, in case it got an optgroup parent.

Add two more pure-virtual functions to OptionElement: setSelectedState(bool) & value().
These aren't used outside of html/ at the moment (unlike the other pure-virtual functions
used by RenderMenuList/RenderListBox) - but they will be used by SelectElement, once it exists.

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

WebCore/ChangeLog
WebCore/dom/OptionElement.cpp
WebCore/dom/OptionElement.h
WebCore/html/HTMLOptionElement.cpp
WebCore/html/HTMLOptionElement.h
WebCore/html/HTMLSelectElement.cpp
WebCore/rendering/RenderListBox.cpp
WebCore/rendering/RenderMenuList.cpp

index 80f3901..5573527 100644 (file)
@@ -1,3 +1,54 @@
+2009-01-22  Nikolas Zimmermann  <nikolas.zimmermann@torchmobile.com>
+
+        Reviewed by Eric Seidel.
+
+        Fixes: https://bugs.webkit.org/show_bug.cgi?id=23465
+
+        Further enhancments to share code between HTMLOptionElement and the upcoming WMLOptionElement.
+
+        Rename optionText() to textIndentedToRespectGroupLabel() in (HTML)OptionElement, as it fits better.
+        optionText() returns the options text prefixed with some spaces, in case it got an optgroup parent.
+
+        Add two more pure-virtual functions to OptionElement: setSelectedState(bool) & value().
+        These aren't used outside of html/ at the moment (unlike the other pure-virtual functions
+        used by RenderMenuList/RenderListBox) - but they will be used by SelectElement, once it exists.
+
+        * dom/OptionElement.cpp:
+        (WebCore::OptionElement::setSelectedState):
+        (WebCore::OptionElement::collectOptionText):
+        (WebCore::OptionElement::collectOptionTextRespectingGroupLabel):
+        (WebCore::OptionElement::collectOptionValue):
+        (WebCore::OptionElementData::OptionElementData):
+        (WebCore::OptionElementData::~OptionElementData):
+        * dom/OptionElement.h:
+        (WebCore::OptionElementData::element):
+        (WebCore::OptionElementData::value):
+        (WebCore::OptionElementData::setValue):
+        (WebCore::OptionElementData::label):
+        (WebCore::OptionElementData::setLabel):
+        (WebCore::OptionElementData::selected):
+        (WebCore::OptionElementData::setSelected):
+        * html/HTMLOptionElement.cpp:
+        (WebCore::HTMLOptionElement::HTMLOptionElement):
+        (WebCore::HTMLOptionElement::text):
+        (WebCore::HTMLOptionElement::parseMappedAttribute):
+        (WebCore::HTMLOptionElement::value):
+        (WebCore::HTMLOptionElement::selected):
+        (WebCore::HTMLOptionElement::setSelected):
+        (WebCore::HTMLOptionElement::setSelectedState):
+        (WebCore::HTMLOptionElement::label):
+        (WebCore::HTMLOptionElement::textIndentedToRespectGroupLabel):
+        * html/HTMLOptionElement.h:
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::typeAheadFind):
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::updateFromElement):
+        (WebCore::RenderListBox::paintItemForeground):
+        * rendering/RenderMenuList.cpp:
+        (WebCore::RenderMenuList::updateOptionsWidth):
+        (WebCore::RenderMenuList::setTextFromOption):
+        (WebCore::RenderMenuList::itemText):
+
 2009-01-22  Chris Fleizach  <cfleizach@apple.com>
 
         Reviewed by Justin Garcia.
index ba5ded7..eeeac93 100644 (file)
 #include "config.h"
 #include "OptionElement.h"
 
+#include "Document.h"
 #include "Element.h"
 #include "HTMLNames.h"
 #include "HTMLOptionElement.h"
+#include "OptionGroupElement.h"
+#include "ScriptElement.h"
 #include <wtf/Assertions.h>
 
 // FIXME: Activate code once WMLOptionElement is available
 
 namespace WebCore {
 
+void OptionElement::setSelectedState(OptionElementData& data, bool selected)
+{
+    if (data.selected() == selected)
+        return;
+
+    data.setSelected(selected);
+    data.element()->setChanged();
+}
+
+String OptionElement::collectOptionText(const OptionElementData& data, Document* document)
+{
+    String text;
+
+    // WinIE does not use the label attribute, so as a quirk, we ignore it.
+    if (!document->inCompatMode())
+        text = data.label();
+
+    if (text.isEmpty()) {
+        Node* n = data.element()->firstChild();
+        while (n) {
+            if (n->nodeType() == Node::TEXT_NODE || n->nodeType() == Node::CDATA_SECTION_NODE)
+                text += n->nodeValue();
+
+            // skip script content
+            if (n->isElementNode() && toScriptElement(static_cast<Element*>(n)))
+                n = n->traverseNextSibling(data.element());
+            else
+                n = n->traverseNextNode(data.element());
+        }
+    }
+
+    text = document->displayStringModifiedByEncoding(text);
+
+    // In WinIE, leading and trailing whitespace is ignored in options and optgroups. We match this behavior.
+    text = text.stripWhiteSpace();
+
+    // We want to collapse our whitespace too.  This will match other browsers.
+    text = text.simplifyWhiteSpace();
+    return text;
+}
+
+String OptionElement::collectOptionTextRespectingGroupLabel(const OptionElementData& data, Document* document)
+{
+    Element* parentElement = static_cast<Element*>(data.element()->parentNode());
+    if (parentElement && optionGroupElementForElement(parentElement))
+        return "    " + collectOptionText(data, document);
+
+    return collectOptionText(data, document);
+}
+
+String OptionElement::collectOptionValue(const OptionElementData& data, Document* document)
+{
+    String value = data.value();
+    if (!value.isNull())
+        return value;
+
+    // Use the text if the value wasn't set.
+    return collectOptionText(data, document).stripWhiteSpace();
+}
+
+// OptionElementData
+OptionElementData::OptionElementData(Element* element)
+    : m_element(element)
+    , m_selected(false)
+{
+    ASSERT(m_element);
+}
+
+OptionElementData::~OptionElementData()
+{
+}
+
 OptionElement* optionElementForElement(Element* element)
 {
     if (element->isHTMLElement() && element->hasTagName(HTMLNames::optionTag))
index 2ba23b3..3cb6953 100644 (file)
 #ifndef OptionElement_h
 #define OptionElement_h
 
+#include "PlatformString.h"
+
 namespace WebCore {
 
 class Element;
-class String;
+class Document;
+class OptionElementData;
 
 class OptionElement {
 public:
     virtual ~OptionElement() { }
 
     virtual bool selected() const = 0;
-    virtual String optionText() const = 0;
+    virtual void setSelectedState(bool) = 0;
+
+    virtual String textIndentedToRespectGroupLabel() const = 0;
+    virtual String value() const = 0;
 
 protected:
     OptionElement() { }
+
+    static void setSelectedState(OptionElementData&, bool selected);
+    static String collectOptionText(const OptionElementData&, Document*);
+    static String collectOptionTextRespectingGroupLabel(const OptionElementData&, Document*);
+    static String collectOptionValue(const OptionElementData&, Document*);
+};
+
+// HTML/WMLOptionElement hold this struct as member variable
+// and pass it to the static helper functions in OptionElement
+class OptionElementData {
+public:
+    OptionElementData(Element*);
+    ~OptionElementData();
+
+    Element* element() const { return m_element; }
+
+    String value() const { return m_value; }
+    void setValue(const String& value) { m_value = value; }
+
+    String label() const { return m_label; }
+    void setLabel(const String& label) { m_label = label; }
+
+    bool selected() const { return m_selected; }
+    void setSelected(bool selected) { m_selected = selected; }
+
+private:
+    Element* m_element;
+    String m_value;
+    String m_label;
+    bool m_selected;
 };
 
 OptionElement* optionElementForElement(Element*);
index 83087da..085019f 100644 (file)
@@ -42,7 +42,7 @@ using namespace HTMLNames;
 
 HTMLOptionElement::HTMLOptionElement(const QualifiedName& tagName, Document* doc, HTMLFormElement* f)
     : HTMLFormControlElement(tagName, doc, f)
-    , m_selected(false)
+    , m_data(this)
     , m_style(0)
 {
     ASSERT(hasTagName(optionTag));
@@ -79,32 +79,7 @@ const AtomicString& HTMLOptionElement::type() const
 
 String HTMLOptionElement::text() const
 {
-    String text;
-
-    // WinIE does not use the label attribute, so as a quirk, we ignore it.
-    if (!document()->inCompatMode())
-        text = getAttribute(labelAttr);
-
-    if (text.isEmpty()) {
-        const Node* n = firstChild();
-        while (n) {
-            if (n->nodeType() == TEXT_NODE || n->nodeType() == CDATA_SECTION_NODE)
-                text += n->nodeValue();
-            // skip script content
-            if (n->isElementNode() && n->hasTagName(HTMLNames::scriptTag))
-                n = n->traverseNextSibling(this);
-            else
-                n = n->traverseNextNode(this);
-        }
-    }
-
-    text = document()->displayStringModifiedByEncoding(text);
-    // In WinIE, leading and trailing whitespace is ignored in options and optgroups. We match this behavior.
-    text = text.stripWhiteSpace();
-    // We want to collapse our whitespace too.  This will match other browsers.
-    text = text.simplifyWhiteSpace();
-
-    return text;
+    return OptionElement::collectOptionText(m_data, document());
 }
 
 void HTMLOptionElement::setText(const String &text, ExceptionCode& ec)
@@ -150,19 +125,18 @@ int HTMLOptionElement::index() const
 void HTMLOptionElement::parseMappedAttribute(MappedAttribute *attr)
 {
     if (attr->name() == selectedAttr)
-        m_selected = (!attr->isNull());
+        m_data.setSelected(!attr->isNull());
     else if (attr->name() == valueAttr)
-        m_value = attr->value();
+        m_data.setValue(attr->value());
+    else if (attr->name() == labelAttr)
+        m_data.setLabel(attr->value());
     else
         HTMLFormControlElement::parseMappedAttribute(attr);
 }
 
 String HTMLOptionElement::value() const
 {
-    if ( !m_value.isNull() )
-        return m_value;
-    // Use the text if the value wasn't set.
-    return text().stripWhiteSpace();
+    return OptionElement::collectOptionValue(m_data, document());
 }
 
 void HTMLOptionElement::setValue(const String& value)
@@ -170,21 +144,25 @@ void HTMLOptionElement::setValue(const String& value)
     setAttribute(valueAttr, value);
 }
 
+bool HTMLOptionElement::selected() const
+{
+    return m_data.selected();
+}
+
 void HTMLOptionElement::setSelected(bool selected)
 {
-    if (m_selected == selected)
+    if (m_data.selected() == selected)
         return;
+
+    OptionElement::setSelectedState(m_data, selected);
+
     if (HTMLSelectElement* select = ownerSelectElement())
         select->setSelectedIndex(selected ? index() : -1, false);
-    m_selected = selected;
 }
 
 void HTMLOptionElement::setSelectedState(bool selected)
 {
-    if (m_selected == selected)
-        return;
-    m_selected = selected;
-    setChanged();
+    OptionElement::setSelectedState(m_data, selected);
 }
 
 void HTMLOptionElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
@@ -219,7 +197,7 @@ void HTMLOptionElement::setDefaultSelected(bool b)
 
 String HTMLOptionElement::label() const
 {
-    return getAttribute(labelAttr);
+    return m_data.label();
 }
 
 void HTMLOptionElement::setLabel(const String& value)
@@ -237,12 +215,9 @@ RenderStyle* HTMLOptionElement::nonRendererRenderStyle() const
     return m_style.get(); 
 }
 
-String HTMLOptionElement::optionText() const
+String HTMLOptionElement::textIndentedToRespectGroupLabel() const
 {
-    if (parentNode() && parentNode()->hasTagName(optgroupTag))
-        return "    " + text();
-
-    return text();
+    return OptionElement::collectOptionTextRespectingGroupLabel(m_data, document());
 }
 
 bool HTMLOptionElement::disabled() const
index 1ee2925..39b857c 100644 (file)
@@ -56,12 +56,12 @@ public:
     int index() const;
     virtual void parseMappedAttribute(MappedAttribute*);
 
-    String value() const;
+    virtual String value() const;
     void setValue(const String&);
 
-    virtual bool selected() const { return m_selected; }
+    virtual bool selected() const;
     void setSelected(bool);
-    void setSelectedState(bool);
+    virtual void setSelectedState(bool);
 
     HTMLSelectElement* ownerSelectElement() const;
 
@@ -73,7 +73,7 @@ public:
     String label() const;
     void setLabel(const String&);
 
-    virtual String optionText() const;
+    virtual String textIndentedToRespectGroupLabel() const;
 
     virtual bool disabled() const;
     
@@ -82,9 +82,8 @@ public:
     
 private:
     virtual RenderStyle* nonRendererRenderStyle() const;
-    
-    String m_value;
-    bool m_selected;
+
+    OptionElementData m_data;
     RefPtr<RenderStyle> m_style;
 };
 
index f347698..b8e2608 100644 (file)
@@ -979,7 +979,8 @@ void HTMLSelectElement::typeAheadFind(KeyboardEvent* event)
         if (!items[index]->hasTagName(optionTag) || items[index]->disabled())
             continue;
 
-        if (stripLeadingWhiteSpace(static_cast<HTMLOptionElement*>(items[index])->optionText()).startsWith(prefix, false)) {
+        String text = static_cast<HTMLOptionElement*>(items[index])->textIndentedToRespectGroupLabel();
+        if (stripLeadingWhiteSpace(text).startsWith(prefix, false)) {
             setSelectedIndex(listToOptionIndex(index));
             if(!usesMenuList())
                 listBoxOnChange();
index f0f3493..f0cc663 100644 (file)
@@ -105,7 +105,7 @@ void RenderListBox::updateFromElement()
             String text;
             Font itemFont = style()->font();
             if (OptionElement* optionElement = optionElementForElement(element))
-                text = optionElement->optionText();
+                text = optionElement->textIndentedToRespectGroupLabel();
             else if (OptionGroupElement* optionGroupElement = optionGroupElementForElement(element)) {
                 text = optionGroupElement->groupLabelText();
                 FontDescription d = itemFont.fontDescription();
@@ -300,7 +300,7 @@ void RenderListBox::paintItemForeground(PaintInfo& paintInfo, int tx, int ty, in
 
     String itemText;
     if (optionElement)
-        itemText = optionElement->optionText();
+        itemText = optionElement->textIndentedToRespectGroupLabel();
     else if (OptionGroupElement* optionGroupElement = optionGroupElementForElement(element))
         itemText = optionGroupElement->groupLabelText();      
 
index a4c1767..31a310a 100644 (file)
@@ -146,7 +146,7 @@ void RenderMenuList::updateOptionsWidth()
         if (!optionElement)
             continue;
 
-        String text = optionElement->optionText();
+        String text = optionElement->textIndentedToRespectGroupLabel();
         if (!text.isEmpty())
             maxOptionWidth = max(maxOptionWidth, style()->font().floatWidth(text));
     }
@@ -182,7 +182,7 @@ void RenderMenuList::setTextFromOption(int optionIndex)
     String text = "";
     if (i >= 0 && i < size) {
         if (OptionElement* optionElement = optionElementForElement(listItems[i]))
-            text = optionElement->optionText();
+            text = optionElement->textIndentedToRespectGroupLabel();
     }
 
     setText(text.stripWhiteSpace());
@@ -308,7 +308,7 @@ String RenderMenuList::itemText(unsigned listIndex) const
     if (OptionGroupElement* optionGroupElement = optionGroupElementForElement(element))
         return optionGroupElement->groupLabelText();
     else if (OptionElement* optionElement = optionElementForElement(element))
-        return optionElement->optionText();
+        return optionElement->textIndentedToRespectGroupLabel();
     return String();
 }