[Forms] The option element should not be form associated element.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Mar 2012 04:46:00 +0000 (04:46 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Mar 2012 04:46:00 +0000 (04:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=79764

Patch by Yoshifumi Inoue <yosin@chromium.org> on 2012-03-20
Reviewed by Kent Tamura.

Source/WebCore:

This patch changes base class of HTMLOptionELement to HTMLElement
from HTMLFormControlElement for saving memory space and iteration
time of extra "option" elements in HTMLFormElement::m_formAssociatedElements
and matching the HTML5 specification for ease of maintenance.

This patch changes behavior of handling of CSS pseudo classes "invalid"
and "valid". The "option" elements no longer use these CSS pseudo classes
as HTML5 specification. This bug was filed in https://bugs.webkit.org/show_bug.cgi?id=80088

Changes of TextIterator is lead by usage of isFormControlElement. This
changes will be replaced with more meaningful predicate as part of
https://bugs.webkit.org/show_bug.cgi?id=80381

No new tests but updated select-live-pseudo-selectors.html test.

* css/CSSStyleSelector.cpp:
(WebCore::CSSStyleSelector::canShareStyleWithElement): Added checking of the "option" element and returns false as HTMLFormControlElement.
* css/SelectorChecker.cpp:
(WebCore::SelectorChecker::checkOneSelector): Removed isFormControlElement for PseudoDisabled and PseudoChecked.
* html/HTMLKeygenElement.cpp:
(WebCore::HTMLKeygenElement::HTMLKeygenElement): Removed form parameter of call site of HTMLOptionElement::create.
* html/HTMLOptionElement.cpp:
(WebCore::HTMLOptionElement::HTMLOptionElement): Removed form parameter which no longer needed. Changed base class in initialization list. Added m_disabled initialization.
(WebCore::HTMLOptionElement::create): Removed form parameter which no longer needed.
(WebCore::HTMLOptionElement::attach): Changeid base class.
(WebCore::HTMLOptionElement::detach): Changed base class.
(WebCore::HTMLOptionElement::parseAttribute): Changed base class. Added "disabled" attribute handling.
(WebCore::HTMLOptionElement::childrenChanged): Changed base class.
(WebCore::HTMLOptionElement::insertedIntoTree): Changed base class.
* html/HTMLOptionElement.h:
(HTMLOptionElement): Added new member variable m_disabled which was in HTMLFormControlElement.
(WebCore::HTMLOptionElement::ownElementDisabled): Changed for using m_disabled.
* html/HTMLTagNames.in: Removed constructorNeedsFormElement for the "option" element, which was used for passing form parameter to create function.

LayoutTests:

This patch fixes a bug in select-live-pseudo-selectors.js, adds
assertions to improve coverage, and updates test expectation for
behavior changes (makes the "option" element uses CSS pseudo class
":valid".)

* fast/forms/resources/select-live-pseudo-selectors.js:
(mouseDownOnSelect): Copied from listbox-selection.html for replacing broken simulateClick which used position and size of the "option" element, but these values are zero. Note: five files use mouseDownOnSelect. We'll share this function in future tracked by https://bugs.webkit.org/show_bug.cgi?id=81496.
(backgroundOf): Added String parameter support for ease of writing test case.
* fast/forms/select-live-pseudo-selectors-expected.txt: Added check fo background color of the "selection" element. Changed expected color of the "option" element because the "option" element doesn't support CSS pseudo class ":valid". This also covers bug 80088.

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/resources/select-live-pseudo-selectors.js
LayoutTests/fast/forms/select-live-pseudo-selectors-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/css/CSSStyleSelector.cpp
Source/WebCore/css/SelectorChecker.cpp
Source/WebCore/html/HTMLKeygenElement.cpp
Source/WebCore/html/HTMLOptionElement.cpp
Source/WebCore/html/HTMLOptionElement.h
Source/WebCore/html/HTMLTagNames.in

index 311264c..d53ed24 100644 (file)
@@ -1,3 +1,20 @@
+2012-03-20  Yoshifumi Inoue  <yosin@chromium.org>
+
+        [Forms] The option element should not be form associated element.
+        https://bugs.webkit.org/show_bug.cgi?id=79764
+
+        Reviewed by Kent Tamura.
+
+        This patch fixes a bug in select-live-pseudo-selectors.js, adds
+        assertions to improve coverage, and updates test expectation for
+        behavior changes (makes the "option" element uses CSS pseudo class
+        ":valid".)
+
+        * fast/forms/resources/select-live-pseudo-selectors.js:
+        (mouseDownOnSelect): Copied from listbox-selection.html for replacing broken simulateClick which used position and size of the "option" element, but these values are zero. Note: five files use mouseDownOnSelect. We'll share this function in future tracked by https://bugs.webkit.org/show_bug.cgi?id=81496.
+        (backgroundOf): Added String parameter support for ease of writing test case.
+        * fast/forms/select-live-pseudo-selectors-expected.txt: Added check fo background color of the "selection" element. Changed expected color of the "option" element because the "option" element doesn't support CSS pseudo class ":valid". This also covers bug 80088.
+
 2012-03-20  Dan Bernstein  <mitz@apple.com>
 
         Skipped fast/selectors/selection-window-inactive.html in WebKit2 because of
index 6380bc2..35c3a5b 100644 (file)
@@ -6,17 +6,18 @@ document.body.appendChild(form);
 var nonForm = document.createElement('div');
 document.body.appendChild(nonForm);
 
-function simulateClick(element) {
-    var rect = element.getBoundingClientRect();
-    var x = rect.left + rect.width / 2;
-    var y = rect.top + rect.height / 2;
-
-    if (!window.eventSender) {
-        return;
+function mouseDownOnSelect(selId, index, modifier) {
+    var sl = document.getElementById(selId);
+    var itemHeight = Math.floor(sl.offsetHeight / sl.size);
+    var border = 1;
+    var y = border + index * itemHeight;
+
+    sl.focus();
+    if (window.eventSender) {
+        eventSender.mouseMoveTo(sl.offsetLeft + border, sl.offsetTop + y - window.pageYOffset);
+        eventSender.mouseDown(0, [modifier]);
+        eventSender.mouseUp(0, [modifier]);
     }
-    eventSender.mouseMoveTo(x, y);
-    eventSender.mouseDown();
-    eventSender.mouseUp();
 }
 
 function makeInvalid() {
@@ -50,6 +51,8 @@ function removeOption(option, select) {
 }
 
 function backgroundOf(el) {
+    if (typeof(el) == 'string')
+        el = document.getElementById(el);
     return document.defaultView.getComputedStyle(el, null).getPropertyValue('background-color');
 }
 
@@ -58,6 +61,7 @@ var invalidColor = 'rgb(255, 0, 0)';
 var normalColor = 'rgb(255, 255, 255)';
 var disabledColor = 'rgb(0, 0, 0)';
 var readOnlyColor = 'rgb(0, 255, 0)'
+var transparentColor = 'rgba(0, 0, 0, 0)';
 var validColor = 'rgb(0, 0, 255)';
 
 // --------------------------------
@@ -102,7 +106,7 @@ shouldBe(elBackground, 'invalidColor');
 // --------------------------------
 
 debug('Change the values of select elements without explicit initializing values by clicking:');
-form.innerHTML = '<select id="select-multiple" multiple required>' +
+form.innerHTML = '<select id="select-multiple" multiple required size="4">' +
 '  <option id="multiple-empty">empty</option>' +
 '  <option id="multiple-another">another</option>' +
 '</select>' +
@@ -110,14 +114,12 @@ form.innerHTML = '<select id="select-multiple" multiple required>' +
 '  <option id="size4-empty">empty</option>' +
 '  <option id="size4-another">another</option>' +
 '</select>';
-var selectMultiple = document.getElementById("multiple-empty");
-selectMultiple.focus();
-simulateClick(selectMultiple);
-var selectSize4 = document.getElementById("size4-empty");
-selectSize4.focus();
-simulateClick(selectSize4);
-shouldBe('backgroundOf(selectMultiple)', 'validColor');
-shouldBe('backgroundOf(selectSize4)', 'validColor');
+mouseDownOnSelect('select-multiple', 0);
+mouseDownOnSelect('select-size4', 0);
+shouldBe('backgroundOf("select-multiple")', 'validColor');
+shouldBe('backgroundOf("multiple-empty")', 'transparentColor');
+shouldBe('backgroundOf("select-size4")', 'validColor');
+shouldBe('backgroundOf("size4-empty")', 'transparentColor');
 
 debug('Change the value with a placeholder label option:');
 el = makeInvalid();
index 8197ac4..e226536 100644 (file)
@@ -15,8 +15,10 @@ Inside/outside of a form:
 PASS backgroundOf(el) is invalidColor
 PASS backgroundOf(el) is invalidColor
 Change the values of select elements without explicit initializing values by clicking:
-PASS backgroundOf(selectMultiple) is validColor
-PASS backgroundOf(selectSize4) is validColor
+PASS backgroundOf("select-multiple") is validColor
+PASS backgroundOf("multiple-empty") is transparentColor
+PASS backgroundOf("select-size4") is validColor
+PASS backgroundOf("size4-empty") is transparentColor
 Change the value with a placeholder label option:
 PASS backgroundOf(el) is validColor
 PASS backgroundOf(el) is invalidColor
index 590d20d..1dbdb19 100644 (file)
@@ -1,3 +1,44 @@
+2012-03-20  Yoshifumi Inoue  <yosin@chromium.org>
+
+        [Forms] The option element should not be form associated element.
+        https://bugs.webkit.org/show_bug.cgi?id=79764
+
+        Reviewed by Kent Tamura.
+
+        This patch changes base class of HTMLOptionELement to HTMLElement
+        from HTMLFormControlElement for saving memory space and iteration
+        time of extra "option" elements in HTMLFormElement::m_formAssociatedElements
+        and matching the HTML5 specification for ease of maintenance.
+
+        This patch changes behavior of handling of CSS pseudo classes "invalid"
+        and "valid". The "option" elements no longer use these CSS pseudo classes
+        as HTML5 specification. This bug was filed in https://bugs.webkit.org/show_bug.cgi?id=80088
+
+        Changes of TextIterator is lead by usage of isFormControlElement. This
+        changes will be replaced with more meaningful predicate as part of
+        https://bugs.webkit.org/show_bug.cgi?id=80381
+
+        No new tests but updated select-live-pseudo-selectors.html test.
+
+        * css/CSSStyleSelector.cpp:
+        (WebCore::CSSStyleSelector::canShareStyleWithElement): Added checking of the "option" element and returns false as HTMLFormControlElement.
+        * css/SelectorChecker.cpp:
+        (WebCore::SelectorChecker::checkOneSelector): Removed isFormControlElement for PseudoDisabled and PseudoChecked.
+        * html/HTMLKeygenElement.cpp:
+        (WebCore::HTMLKeygenElement::HTMLKeygenElement): Removed form parameter of call site of HTMLOptionElement::create.
+        * html/HTMLOptionElement.cpp:
+        (WebCore::HTMLOptionElement::HTMLOptionElement): Removed form parameter which no longer needed. Changed base class in initialization list. Added m_disabled initialization.
+        (WebCore::HTMLOptionElement::create): Removed form parameter which no longer needed.
+        (WebCore::HTMLOptionElement::attach): Changeid base class.
+        (WebCore::HTMLOptionElement::detach): Changed base class.
+        (WebCore::HTMLOptionElement::parseAttribute): Changed base class. Added "disabled" attribute handling.
+        (WebCore::HTMLOptionElement::childrenChanged): Changed base class.
+        (WebCore::HTMLOptionElement::insertedIntoTree): Changed base class.
+        * html/HTMLOptionElement.h:
+        (HTMLOptionElement): Added new member variable m_disabled which was in HTMLFormControlElement.
+        (WebCore::HTMLOptionElement::ownElementDisabled): Changed for using m_disabled.
+        * html/HTMLTagNames.in: Removed constructorNeedsFormElement for the "option" element, which was used for passing form parameter to create function.
+
 2012-03-20  Xiaomei Ji  <xji@chromium.org>
 
         Crash introduced in r110965.
index 97178ea..7a2a54f 100644 (file)
@@ -65,6 +65,7 @@
 #include "HTMLElement.h"
 #include "HTMLInputElement.h"
 #include "HTMLNames.h"
+#include "HTMLOptionElement.h"
 #include "HTMLProgressElement.h"
 #include "HTMLStyleElement.h"
 #include "HTMLTextAreaElement.h"
@@ -1344,6 +1345,18 @@ bool CSSStyleSelector::canShareStyleWithElement(StyledElement* element) const
     }
 #endif
 
+    if (element->hasTagName(optionTag)) {
+        if (!m_element->hasTagName(optionTag))
+            return false;
+
+        HTMLOptionElement* thisOptionElement = static_cast<HTMLOptionElement*>(element);
+        HTMLOptionElement* otherOptionElement = static_cast<HTMLOptionElement*>(m_element);
+        if (thisOptionElement->isEnabledFormControl() != otherOptionElement->isEnabledFormControl())
+            return false;
+        if (thisOptionElement->selected() != otherOptionElement->selected())
+            return false;
+    }
+
     bool isControl = element->isFormControlElement();
 
     if (isControl != m_element->isFormControlElement())
index 0864915..bf3ffd5 100644 (file)
@@ -1052,7 +1052,7 @@ bool SelectorChecker::checkOneSelector(const SelectorCheckingContext& context, P
         case CSSSelector::PseudoDefault:
             return element && element->isDefaultButtonForForm();
         case CSSSelector::PseudoDisabled:
-            if (element && element->isFormControlElement())
+            if (element && (element->isFormControlElement() || element->hasTagName(optionTag)))
                 return !element->isEnabledFormControl();
             break;
         case CSSSelector::PseudoReadOnly:
@@ -1079,7 +1079,7 @@ bool SelectorChecker::checkOneSelector(const SelectorCheckingContext& context, P
             return (element->willValidate() && !element->isValidFormControlElement()) || element->hasUnacceptableValue();
         case CSSSelector::PseudoChecked:
             {
-                if (!element || !element->isFormControlElement())
+                if (!element)
                     break;
                 // Even though WinIE allows checked and indeterminate to co-exist, the CSS selector spec says that
                 // you can't be both checked and indeterminate. We will behave like WinIE behind the scenes and just
index 8053441..3094c93 100644 (file)
@@ -81,7 +81,7 @@ inline HTMLKeygenElement::HTMLKeygenElement(const QualifiedName& tagName, Docume
     RefPtr<HTMLSelectElement> select = KeygenSelectElement::create(document);
     ExceptionCode ec = 0;
     for (size_t i = 0; i < keys.size(); ++i) {
-        RefPtr<HTMLOptionElement> option = HTMLOptionElement::create(document, this->form());
+        RefPtr<HTMLOptionElement> option = HTMLOptionElement::create(document);
         select->appendChild(option, ec);
         option->appendChild(Text::create(document, keys[i]), ec);
     }
index 1696e9b..a9be7c0 100644 (file)
@@ -37,6 +37,7 @@
 #include "NodeRenderStyle.h"
 #include "NodeRenderingContext.h"
 #include "RenderMenuList.h"
+#include "RenderTheme.h"
 #include "ScriptElement.h"
 #include "Text.h"
 #include <wtf/StdLibExtras.h>
@@ -47,21 +48,22 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
-HTMLOptionElement::HTMLOptionElement(const QualifiedName& tagName, Document* document, HTMLFormElement* form)
-    : HTMLFormControlElement(tagName, document, form)
+HTMLOptionElement::HTMLOptionElement(const QualifiedName& tagName, Document* document)
+    : HTMLElement(tagName, document)
+    , m_disabled(false)
     , m_isSelected(false)
 {
     ASSERT(hasTagName(optionTag));
 }
 
-PassRefPtr<HTMLOptionElement> HTMLOptionElement::create(Document* document, HTMLFormElement* form)
+PassRefPtr<HTMLOptionElement> HTMLOptionElement::create(Document* document)
 {
-    return adoptRef(new HTMLOptionElement(optionTag, document, form));
+    return adoptRef(new HTMLOptionElement(optionTag, document));
 }
 
-PassRefPtr<HTMLOptionElement> HTMLOptionElement::create(const QualifiedName& tagName, Document* document, HTMLFormElement* form)
+PassRefPtr<HTMLOptionElement> HTMLOptionElement::create(const QualifiedName& tagName, Document* document)
 {
-    return adoptRef(new HTMLOptionElement(tagName, document, form));
+    return adoptRef(new HTMLOptionElement(tagName, document));
 }
 
 PassRefPtr<HTMLOptionElement> HTMLOptionElement::createForJSConstructor(Document* document, const String& data, const String& value,
@@ -89,13 +91,13 @@ void HTMLOptionElement::attach()
 {
     if (parentNode()->renderStyle())
         setRenderStyle(styleForRenderer());
-    HTMLFormControlElement::attach();
+    HTMLElement::attach();
 }
 
 void HTMLOptionElement::detach()
 {
     m_style.clear();
-    HTMLFormControlElement::detach();
+    HTMLElement::detach();
 }
 
 bool HTMLOptionElement::supportsFocus() const
@@ -109,12 +111,6 @@ bool HTMLOptionElement::isFocusable() const
     return supportsFocus() && renderStyle() && renderStyle()->display() != NONE;
 }
 
-const AtomicString& HTMLOptionElement::formControlType() const
-{
-    DEFINE_STATIC_LOCAL(const AtomicString, option, ("option"));
-    return option;
-}
-
 String HTMLOptionElement::text() const
 {
     Document* document = this->document();
@@ -191,7 +187,15 @@ int HTMLOptionElement::index() const
 
 void HTMLOptionElement::parseAttribute(Attribute* attr)
 {
-    if (attr->name() == selectedAttr) {
+    if (attr->name() == disabledAttr) {
+        bool oldDisabled = m_disabled;
+        m_disabled = !attr->isNull();
+        if (oldDisabled != m_disabled) {
+            setNeedsStyleRecalc();
+            if (renderer() && renderer()->style()->hasAppearance())
+                renderer()->theme()->stateChanged(renderer(), EnabledState);
+        }
+    } else if (attr->name() == selectedAttr) {
         // FIXME: This doesn't match what the HTML specification says.
         // The specification implies that removing the selected attribute or
         // changing the value of a selected attribute that is already present
@@ -200,7 +204,7 @@ void HTMLOptionElement::parseAttribute(Attribute* attr)
         // case; we'd need to do the other work from the setSelected function.
         m_isSelected = !attr->isNull();
     } else
-        HTMLFormControlElement::parseAttribute(attr);
+        HTMLElement::parseAttribute(attr);
 }
 
 String HTMLOptionElement::value() const
@@ -247,7 +251,7 @@ void HTMLOptionElement::childrenChanged(bool changedByParser, Node* beforeChange
 {
     if (HTMLSelectElement* select = ownerSelectElement())
         select->optionElementChildrenChanged();
-    HTMLFormControlElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
+    HTMLElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
 }
 
 HTMLSelectElement* HTMLOptionElement::ownerSelectElement() const
@@ -315,7 +319,7 @@ void HTMLOptionElement::insertedIntoTree(bool deep)
         select->scrollToSelection();
     }
 
-    HTMLFormControlElement::insertedIntoTree(deep);
+    HTMLElement::insertedIntoTree(deep);
 }
 
 String HTMLOptionElement::collectOptionInnerText() const
index 2eedadf..1433a82 100644 (file)
 #ifndef HTMLOptionElement_h
 #define HTMLOptionElement_h
 
-#include "HTMLFormControlElement.h"
+#include "HTMLElement.h"
 
 namespace WebCore {
 
 class HTMLSelectElement;
 
-class HTMLOptionElement : public HTMLFormControlElement {
+class HTMLOptionElement : public HTMLElement {
 public:
-    static PassRefPtr<HTMLOptionElement> create(Document*, HTMLFormElement*);
-    static PassRefPtr<HTMLOptionElement> create(const QualifiedName&, Document*, HTMLFormElement*);
+    static PassRefPtr<HTMLOptionElement> create(Document*);
+    static PassRefPtr<HTMLOptionElement> create(const QualifiedName&, Document*);
     static PassRefPtr<HTMLOptionElement> createForJSConstructor(Document*, const String& data, const String& value,
        bool defaultSelected, bool selected, ExceptionCode&);
 
@@ -54,7 +54,8 @@ public:
     String label() const;
     void setLabel(const String&);
 
-    bool ownElementDisabled() const { return HTMLFormControlElement::disabled(); }
+    virtual bool isEnabledFormControl() const OVERRIDE { return !disabled(); }
+    bool ownElementDisabled() const { return m_disabled; }
 
     virtual bool disabled() const;
 
@@ -63,7 +64,7 @@ public:
     void setSelectedState(bool);
 
 private:
-    HTMLOptionElement(const QualifiedName&, Document*, HTMLFormElement* = 0);
+    HTMLOptionElement(const QualifiedName&, Document*);
 
     virtual bool supportsFocus() const;
     virtual bool isFocusable() const;
@@ -72,8 +73,6 @@ private:
     virtual void detach();
     virtual void setRenderStyle(PassRefPtr<RenderStyle>);
 
-    virtual const AtomicString& formControlType() const;
-
     virtual void parseAttribute(Attribute*) OVERRIDE;
 
     virtual void insertedIntoTree(bool);
@@ -87,6 +86,7 @@ private:
 
     String m_value;
     String m_label;
+    bool m_disabled;
     bool m_isSelected;
     RefPtr<RenderStyle> m_style;
 };
index 74e2408..4997653 100644 (file)
@@ -92,7 +92,7 @@ nolayer interfaceName=HTMLElement
 object constructorNeedsFormElement, constructorNeedsCreatedByParser
 ol interfaceName=HTMLOListElement
 optgroup interfaceName=HTMLOptGroupElement
-option constructorNeedsFormElement
+option
 output constructorNeedsFormElement
 shadow interfaceName=HTMLShadowElement, conditional=SHADOW_DOM, runtimeConditional=shadowDOM
 p interfaceName=HTMLParagraphElement