Changeset 40130 in webkit

Timestamp:

Jan 22, 2009 12:40:18 PM (15 years ago)

Author:

Nikolas Zimmermann

Message:

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.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/OptionElement.cpp (modified) (2 diffs)
• dom/OptionElement.h (modified) (2 diffs)
• html/HTMLOptionElement.cpp (modified) (7 diffs)
• html/HTMLOptionElement.h (modified) (3 diffs)
• html/HTMLSelectElement.cpp (modified) (1 diff)
• rendering/RenderListBox.cpp (modified) (2 diffs)
• rendering/RenderMenuList.cpp (modified) (3 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/WebCore/dom/OptionElement.cpp

@@ -22 +22 @@
 #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>
 
@@ -34 +37 @@
 
 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)

trunk/WebCore/dom/OptionElement.h

@@ -22 +22 @@
 #define OptionElement_h
 
+#include "PlatformString.h"
+
 namespace WebCore {
 
 class Element;
-class String;
+class Document;
+class OptionElementData;
 
 class OptionElement {
@@ -32 +35 @@
 
     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;
 };
 

trunk/WebCore/html/HTMLOptionElement.cpp

@@ -43 +43 @@
 HTMLOptionElement::HTMLOptionElement(const QualifiedName& tagName, Document* doc, HTMLFormElement* f)
     : HTMLFormControlElement(tagName, doc, f)
-    , m_selected(false)
+    , m_data(this)
     , m_style(0)
 {
@@ -80 +80 @@
 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());
 }
 
@@ -151 +126 @@
 {
     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);
@@ -160 +137 @@
 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());
 }
 
@@ -171 +145 @@
 }
 
+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);
 }
 
@@ -220 +198 @@
 String HTMLOptionElement::label() const
 {
-    return getAttribute(labelAttr);
+    return m_data.label();
 }
 
@@ -238 +216 @@
 }
 
-String HTMLOptionElement::optionText() const
-{
-    if (parentNode() && parentNode()->hasTagName(optgroupTag))
-        return "    " + text();
-
-    return text();
+String HTMLOptionElement::textIndentedToRespectGroupLabel() const
+{
+    return OptionElement::collectOptionTextRespectingGroupLabel(m_data, document());
 }
 

trunk/WebCore/html/HTMLOptionElement.h

@@ -57 +57 @@
     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;
@@ -74 +74 @@
     void setLabel(const String&);
 
-    virtual String optionText() const;
+    virtual String textIndentedToRespectGroupLabel() const;
 
     virtual bool disabled() const;
@@ -83 +83 @@
 private:
     virtual RenderStyle* nonRendererRenderStyle() const;
-    
-    String m_value;
-    bool m_selected;
+
+    OptionElementData m_data;
     RefPtr<RenderStyle> m_style;
 };

trunk/WebCore/html/HTMLSelectElement.cpp

@@ -980 +980 @@
             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())

trunk/WebCore/rendering/RenderListBox.cpp

@@ -106 +106 @@
             Font itemFont = style()->font();
             if (OptionElement* optionElement = optionElementForElement(element))
-                text = optionElement->optionText();
+                text = optionElement->textIndentedToRespectGroupLabel();
             else if (OptionGroupElement* optionGroupElement = optionGroupElementForElement(element)) {
                 text = optionGroupElement->groupLabelText();
@@ -301 +301 @@
     String itemText;
     if (optionElement)
-        itemText = optionElement->optionText();
+        itemText = optionElement->textIndentedToRespectGroupLabel();
     else if (OptionGroupElement* optionGroupElement = optionGroupElementForElement(element))
         itemText = optionGroupElement->groupLabelText();      

trunk/WebCore/rendering/RenderMenuList.cpp

@@ -147 +147 @@
             continue;
 
-        String text = optionElement->optionText();
+        String text = optionElement->textIndentedToRespectGroupLabel();
         if (!text.isEmpty())
             maxOptionWidth = max(maxOptionWidth, style()->font().floatWidth(text));
@@ -183 +183 @@
     if (i >= 0 && i < size) {
         if (OptionElement* optionElement = optionElementForElement(listItems[i]))
-            text = optionElement->optionText();
+            text = optionElement->textIndentedToRespectGroupLabel();
     }
 
@@ -309 +309 @@
         return optionGroupElement->groupLabelText();
     else if (OptionElement* optionElement = optionElementForElement(element))
-        return optionElement->optionText();
+        return optionElement->textIndentedToRespectGroupLabel();
     return String();
 }