Changeset 104668 in webkit


Ignore:
Timestamp:
Jan 10, 2012 9:03:23 PM (12 years ago)
Author:
tkent@chromium.org
Message:

Source/WebCore: A radio button not in a document should not join a radio button group
​https://bugs.webkit.org/show_bug.cgi?id=45719

Reviewed by Darin Adler.

As per the standard and other browser behaviors, we should not add a
radio button not in a document to a radio button group.

So, CheckedRadioButton member functions should be called in
insertedIntoDocument() and removedFromDocument(), and they should do
nothing if the specified radio button is not in a document.

This change also fixes a bug that form owner change by 'form' attribute
didn't update radio button groups correctly.

Test: fast/forms/radio/radio-group.html

  • dom/CheckedRadioButtons.cpp:

(WebCore::shouldMakeRadioGroup): A helper function to check <input> state.
(WebCore::CheckedRadioButtons::addButton):

  • Change the argument type: HTMLFormControlElement* -> HTMLInputElement*.
  • Use shouldMakeRadioGroup().

(WebCore::CheckedRadioButtons::removeButton): ditto.

  • dom/CheckedRadioButtons.h: Change the argument types of addButton() and removeButton(): HTMLFormControlElement* -> HTMLInputElement*.
  • html/FormAssociatedElement.cpp: Do not update m_form except in setForm() and formWillBeDestroyed(). This helps us to call registerFormElement() and removeFromElement() correctly, and call willChangeForm()/didChangeForm() hooks correctly.

(WebCore::FormAssociatedElement::FormAssociatedElement):
(WebCore::FormAssociatedElement::~FormAssociatedElement):
(WebCore::FormAssociatedElement::insertedIntoTree):
(WebCore::FormAssociatedElement::removedFromTree):
(WebCore::FormAssociatedElement::setForm):
(WebCore::FormAssociatedElement::willChangeForm):
This virtual function is called before the owner form is updated.
(WebCore::FormAssociatedElement::didChangeForm):
This virtual function is called after the owner form was updated.
(WebCore::FormAssociatedElement::formWillBeDestroyed):

  • Renamaed from formDestroyed()
  • Add calls to willChangeForm() and didChangeForm().

(WebCore::FormAssociatedElement::resetFormOwner): Use setForm().
(WebCore::FormAssociatedElement::formAttributeChanged): ditto.

  • html/FormAssociatedElement.h:
  • html/HTMLFormControlElement.cpp:

(WebCore::HTMLFormControlElement::HTMLFormControlElement): Use setForm().
(WebCore::HTMLFormControlElement::~HTMLFormControlElement):
removeFormElement() is not needed. ~FormAssociatedElement() treats it.
(WebCore::HTMLFormControlElement::parseMappedAttribute):
No need to handle CheckedRadioButtons here. It is handled by HTMLInputElement.
(WebCore::HTMLFormControlElement::insertedIntoTree): ditto.

  • html/HTMLFormElement.cpp:

(WebCore::HTMLFormElement::~HTMLFormElement):

Rename formDestroyed() with willDestroyForm().

(WebCore::HTMLFormElement::registerFormElement):
We don't need to handle radio button groups here.
(WebCore::HTMLFormElement::removeFormElement): ditto.

  • html/HTMLInputElement.cpp:

(WebCore::HTMLInputElement::~HTMLInputElement):
Calls setForm(0) so that it calls correct virtual functions of
HTMLInputElement.
(WebCore::HTMLInputElement::updateCheckedRadioButtons):
Checking attached() was wrong.
(WebCore::HTMLInputElement::willChangeForm):
Remove this radio button from a radio button group for the old form.
(WebCore::HTMLInputElement::didChangeForm):
Add this radio button to a radio button group for the new form.
(WebCore::HTMLInputElement::insertedIntoDocument):
Add CheckedRadioButtons::addButton() call.
(WebCore::HTMLInputElement::removedFromDocument):
Add CheckedRadioButtons::removeButton() call.
(WebCore::HTMLInputElement::didMoveToNewDocument):
We don't need to handle CheckedRadioButton() here because
removedFromDocument() handles it.

  • html/HTMLInputElement.h:
  • html/HTMLObjectElement.cpp:

(WebCore::HTMLObjectElement::HTMLObjectElement): Use setForm().
(WebCore::HTMLObjectElement::~HTMLObjectElement):
removeFormElement() is not needed. ~FormAssociatedElement() treats it.

LayoutTests: A radio button not in a Document should not join a radio button group
​https://bugs.webkit.org/show_bug.cgi?id=45719

Reviewed by Darin Adler.

  • fast/forms/radio/radio-group-expected.txt: Added.
  • fast/forms/radio/radio-group.html: Added.
Location:
trunk
Files:
3 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r104662 r104668  
     12012-01-04  Kent Tamura  <tkent@chromium.org>
     2
     3        A radio button not in a Document should not join a radio button group
     4        https://bugs.webkit.org/show_bug.cgi?id=45719
     5
     6        Reviewed by Darin Adler.
     7
     8        * fast/forms/radio/radio-group-expected.txt: Added.
     9        * fast/forms/radio/radio-group.html: Added.
     10
    1112012-01-10  João Paulo Rechi Vita  <jprvita@openbossa.org>
    212

  • trunk/Source/WebCore/ChangeLog

    r104659 r104668  
     12012-01-04  Kent Tamura  <tkent@chromium.org>
     2
     3        A radio button not in a document should not join a radio button group
     4        https://bugs.webkit.org/show_bug.cgi?id=45719
     5
     6        Reviewed by Darin Adler.
     7
     8        As per the standard and other browser behaviors, we should not add a
     9        radio button not in a document to a radio button group.
     10
     11        So, CheckedRadioButton member functions should be called in
     12        insertedIntoDocument() and removedFromDocument(), and they should do
     13        nothing if the specified radio button is not in a document.
     14
     15        This change also fixes a bug that form owner change by 'form' attribute
     16        didn't update radio button groups correctly.
     17
     18        Test: fast/forms/radio/radio-group.html
     19
     20        * dom/CheckedRadioButtons.cpp:
     21        (WebCore::shouldMakeRadioGroup): A helper function to check <input> state.
     22        (WebCore::CheckedRadioButtons::addButton):
     23        - Change the argument type: HTMLFormControlElement* -> HTMLInputElement*.
     24        - Use shouldMakeRadioGroup().
     25        (WebCore::CheckedRadioButtons::removeButton): ditto.
     26        * dom/CheckedRadioButtons.h:
     27         Change the argument types of addButton() and removeButton():
     28         HTMLFormControlElement* -> HTMLInputElement*.
     29        * html/FormAssociatedElement.cpp:
     30         Do not update m_form except in setForm() and formWillBeDestroyed().
     31         This helps us to call registerFormElement() and removeFromElement()
     32         correctly, and call willChangeForm()/didChangeForm() hooks correctly.
     33        (WebCore::FormAssociatedElement::FormAssociatedElement):
     34        (WebCore::FormAssociatedElement::~FormAssociatedElement):
     35        (WebCore::FormAssociatedElement::insertedIntoTree):
     36        (WebCore::FormAssociatedElement::removedFromTree):
     37        (WebCore::FormAssociatedElement::setForm):
     38        (WebCore::FormAssociatedElement::willChangeForm):
     39        This virtual function is called before the owner form is updated.
     40        (WebCore::FormAssociatedElement::didChangeForm):
     41        This virtual function is called after the owner form was updated.
     42        (WebCore::FormAssociatedElement::formWillBeDestroyed):
     43        - Renamaed from formDestroyed()
     44        - Add calls to willChangeForm() and didChangeForm().
     45        (WebCore::FormAssociatedElement::resetFormOwner): Use setForm().
     46        (WebCore::FormAssociatedElement::formAttributeChanged): ditto.
     47        * html/FormAssociatedElement.h:
     48        * html/HTMLFormControlElement.cpp:
     49        (WebCore::HTMLFormControlElement::HTMLFormControlElement): Use setForm().
     50        (WebCore::HTMLFormControlElement::~HTMLFormControlElement):
     51        removeFormElement() is not needed. ~FormAssociatedElement() treats it.
     52        (WebCore::HTMLFormControlElement::parseMappedAttribute):
     53        No need to handle CheckedRadioButtons here. It is handled by HTMLInputElement.
     54        (WebCore::HTMLFormControlElement::insertedIntoTree): ditto.
     55        * html/HTMLFormElement.cpp:
     56        (WebCore::HTMLFormElement::~HTMLFormElement):
     57         Rename formDestroyed() with willDestroyForm().
     58        (WebCore::HTMLFormElement::registerFormElement):
     59        We don't need to handle radio button groups here.
     60        (WebCore::HTMLFormElement::removeFormElement): ditto.
     61        * html/HTMLInputElement.cpp:
     62        (WebCore::HTMLInputElement::~HTMLInputElement):
     63        Calls setForm(0) so that it calls correct virtual functions of
     64        HTMLInputElement.
     65        (WebCore::HTMLInputElement::updateCheckedRadioButtons):
     66        Checking attached() was wrong.
     67        (WebCore::HTMLInputElement::willChangeForm):
     68        Remove this radio button from a radio button group for the old form.
     69        (WebCore::HTMLInputElement::didChangeForm):
     70        Add this radio button to a radio button group for the new form.
     71        (WebCore::HTMLInputElement::insertedIntoDocument):
     72        Add CheckedRadioButtons::addButton() call.
     73        (WebCore::HTMLInputElement::removedFromDocument):
     74        Add CheckedRadioButtons::removeButton() call.
     75        (WebCore::HTMLInputElement::didMoveToNewDocument):
     76        We don't need to handle CheckedRadioButton() here because
     77        removedFromDocument() handles it.
     78        * html/HTMLInputElement.h:
     79        * html/HTMLObjectElement.cpp:
     80        (WebCore::HTMLObjectElement::HTMLObjectElement): Use setForm().
     81        (WebCore::HTMLObjectElement::~HTMLObjectElement):
     82        removeFormElement() is not needed. ~FormAssociatedElement() treats it.
     83
    1842012-01-10  Kentaro Hara  <haraken@chromium.org>
    285

  • trunk/Source/WebCore/dom/CheckedRadioButtons.cpp

    r94149 r104668  
    2626namespace WebCore {
    2727
    28 void CheckedRadioButtons::addButton(HTMLFormControlElement* element)
     28static inline bool shouldMakeRadioGroup(HTMLInputElement* element)
    2929{
    30     // We only want to add radio buttons.
    31     if (!element->isRadioButton())
     30    return element->isRadioButton() && !element->name().isEmpty() && element->inDocument();
     31}
     32
     33void CheckedRadioButtons::addButton(HTMLInputElement* element)
     34{
     35    if (!shouldMakeRadioGroup(element))
    3236        return;
    3337
    34     // Without a name, there is no group.
    35     if (element->name().isEmpty())
    36         return;
    37 
    38     HTMLInputElement* inputElement = static_cast<HTMLInputElement*>(element);
    39 
    4038    // We only track checked buttons.
    41     if (!inputElement->checked())
     39    if (!element->checked())
    4240        return;
    4341
    … …  
    4543        m_nameToCheckedRadioButtonMap = adoptPtr(new NameToInputMap);
    4644
    47     pair<NameToInputMap::iterator, bool> result = m_nameToCheckedRadioButtonMap->add(element->name().impl(), inputElement);
     45    pair<NameToInputMap::iterator, bool> result = m_nameToCheckedRadioButtonMap->add(element->name().impl(), element);
    4846    if (result.second)
    4947        return;
    5048   
    5149    HTMLInputElement* oldCheckedButton = result.first->second;
    52     if (oldCheckedButton == inputElement)
     50    if (oldCheckedButton == element)
    5351        return;
    5452
    55     result.first->second = inputElement;
     53    result.first->second = element;
    5654    oldCheckedButton->setChecked(false);
    5755}
    … …  
    6765}
    6866
    69 void CheckedRadioButtons::removeButton(HTMLFormControlElement* element)
     67void CheckedRadioButtons::removeButton(HTMLInputElement* element)
    7068{
    7169    if (element->name().isEmpty() || !m_nameToCheckedRadioButtonMap)
    … …  
    7876        return;
    7977   
    80     HTMLInputElement* inputElement = element->toInputElement();
    81     ASSERT_UNUSED(inputElement, inputElement);
    82     ASSERT(inputElement->shouldAppearChecked());
     78    ASSERT(element->shouldAppearChecked());
    8379    ASSERT(element->isRadioButton());
    8480

  • trunk/Source/WebCore/dom/CheckedRadioButtons.h

    r73430 r104668  
    2828namespace WebCore {
    2929
    30 class HTMLFormControlElement;
    3130class HTMLInputElement;
    3231
    3332class CheckedRadioButtons {
    3433public:
    35     void addButton(HTMLFormControlElement*);
    36     void removeButton(HTMLFormControlElement*);
     34    void addButton(HTMLInputElement*);
     35    void removeButton(HTMLInputElement*);
    3736    HTMLInputElement* checkedButtonForGroup(const AtomicString& groupName) const;
    3837

  • trunk/Source/WebCore/html/FormAssociatedElement.cpp

    r103679 r104668  
    3636using namespace HTMLNames;
    3737
    38 FormAssociatedElement::FormAssociatedElement(HTMLFormElement* form)
    39     : m_form(form)
     38FormAssociatedElement::FormAssociatedElement()
     39    : m_form(0)
    4040{
    4141}
    … …  
    4343FormAssociatedElement::~FormAssociatedElement()
    4444{
     45    setForm(0);
    4546}
    4647
    … …  
    7778{
    7879    HTMLElement* element = toHTMLElement(this);
    79     if (element->fastHasAttribute(formAttr)) {
    80         // Resets the form owner at first to make sure the element don't
    81         // associate any form elements when there is no element which has
    82         // the given ID.
    83         if (m_form) {
    84             m_form->removeFormElement(this);
    85             m_form = 0;
    86         }
    87         Element* formElement = element->treeScope()->getElementById(element->fastGetAttribute(formAttr));
    88         if (formElement && formElement->hasTagName(formTag)) {
    89             m_form = static_cast<HTMLFormElement*>(formElement);
    90             m_form->registerFormElement(this);
    91         }
     80    const AtomicString& formId = element->fastGetAttribute(formAttr);
     81    if (!formId.isNull()) {
     82        HTMLFormElement* newForm = 0;
     83        Element* newFormCandidate = element->treeScope()->getElementById(formId);
     84        if (newFormCandidate && newFormCandidate->hasTagName(formTag))
     85            newForm = static_cast<HTMLFormElement*>(newFormCandidate);
     86        setForm(newForm);
    9287        return;
    9388    }
    … …  
    9792        // setting a form, we will already have a non-null value for m_form,
    9893        // and so we don't need to do anything.
    99         m_form = element->findFormAncestor();
    100         if (m_form)
    101             m_form->registerFormElement(this);
     94        setForm(element->findFormAncestor());
    10295    }
    10396}
    … …  
    118111    // Otherwise, null out our form and remove ourselves from the form's list of elements.
    119112    if (m_form && findRoot(element) != findRoot(m_form))
    120         removeFromForm();
     113        setForm(0);
    121114}
    122115
    123 void FormAssociatedElement::removeFromForm()
     116void FormAssociatedElement::setForm(HTMLFormElement* newForm)
    124117{
     118    if (m_form == newForm)
     119        return;
     120    willChangeForm();
     121    if (m_form)
     122        m_form->removeFormElement(this);
     123    m_form = newForm;
     124    if (m_form)
     125        m_form->registerFormElement(this);
     126    didChangeForm();
     127}
     128
     129void FormAssociatedElement::willChangeForm()
     130{
     131}
     132
     133void FormAssociatedElement::didChangeForm()
     134{
     135}
     136
     137void FormAssociatedElement::formWillBeDestroyed()
     138{
     139    ASSERT(m_form);
    125140    if (!m_form)
    126141        return;
    127     m_form->removeFormElement(this);
     142    willChangeForm();
    128143    m_form = 0;
     144    didChangeForm();
    129145}
    130146
    … …  
    136152        if (formId.isNull())
    137153            return;
    138         m_form->removeFormElement(this);
    139154    }
    140     m_form = 0;
     155    HTMLFormElement* newForm = 0;
    141156    if (!formId.isNull() && element->inDocument()) {
    142157        // The HTML5 spec says that the element should be associated with
    … …  
    146161        Element* firstElement = element->treeScope()->getElementById(formId);
    147162        if (firstElement && firstElement->hasTagName(formTag))
    148             m_form = static_cast<HTMLFormElement*>(firstElement);
     163            newForm = static_cast<HTMLFormElement*>(firstElement);
    149164    } else
    150         m_form = element->findFormAncestor();
    151     if (m_form)
    152         m_form->registerFormElement(this);
     165        newForm = element->findFormAncestor();
     166    setForm(newForm);
    153167}
    154168
    … …  
    158172    if (!element->fastHasAttribute(formAttr)) {
    159173        // The form attribute removed. We need to reset form owner here.
    160         if (m_form)
    161             m_form->removeFormElement(this);
    162         m_form = element->findFormAncestor();
    163         if (m_form)
    164             form()->registerFormElement(this);
     174        setForm(element->findFormAncestor());
    165175        element->document()->unregisterFormElementWithFormAttribute(this);
    166176    } else

  • trunk/Source/WebCore/html/FormAssociatedElement.h

    r103679 r104668  
    5454    virtual bool appendFormData(FormDataList&, bool) { return false; }
    5555
    56     virtual void formDestroyed() { m_form = 0; }
     56    void formWillBeDestroyed();
    5757
    5858    void resetFormOwner();
    5959
    6060protected:
    61     FormAssociatedElement(HTMLFormElement*);
     61    FormAssociatedElement();
    6262
    6363    void insertedIntoTree();
    … …  
    6767    void didMoveToNewDocument(Document* oldDocument);
    6868
    69     void setForm(HTMLFormElement* form) { m_form = form; }
    70     void removeFromForm();
     69    void setForm(HTMLFormElement*);
    7170    void formAttributeChanged();
     71
     72    // If you add an override of willChangeForm() or didChangeForm() to a class
     73    // derived from this one, you will need to add a call to setForm(0) to the
     74    // destructor of that class.
     75    virtual void willChangeForm();
     76    virtual void didChangeForm();
    7277
    7378private:

  • trunk/Source/WebCore/html/HTMLFormControlElement.cpp

    r104274 r104668  
    5151HTMLFormControlElement::HTMLFormControlElement(const QualifiedName& tagName, Document* document, HTMLFormElement* form)
    5252    : HTMLElement(tagName, document)
    53     , FormAssociatedElement(form)
    5453    , m_disabled(false)
    5554    , m_readOnly(false)
    … …  
    6261    , m_hasAutofocused(false)
    6362{
    64     if (!this->form())
    65         setForm(findFormAncestor());
    66     if (this->form())
    67         this->form()->registerFormElement(this);
    68 
     63    setForm(form ? form : findFormAncestor());
    6964    setHasCustomWillOrDidRecalcStyle();
    7065}
    … …  
    7267HTMLFormControlElement::~HTMLFormControlElement()
    7368{
    74     if (form())
    75         form()->removeFormElement(this);
    7669}
    7770
    … …  
    109102void HTMLFormControlElement::parseMappedAttribute(Attribute* attr)
    110103{
    111     if (attr->name() == formAttr) {
     104    if (attr->name() == formAttr)
    112105        formAttributeChanged();
    113         if (!form())
    114             document()->checkedRadioButtons().addButton(this);
    115     } else if (attr->name() == disabledAttr) {
     106    else if (attr->name() == disabledAttr) {
    116107        bool oldDisabled = m_disabled;
    117108        m_disabled = !attr->isNull();
    … …  
    206197{
    207198    FormAssociatedElement::insertedIntoTree();
    208     if (!form())
    209         document()->checkedRadioButtons().addButton(this);
    210 
    211199    HTMLElement::insertedIntoTree(deep);
    212200}

  • trunk/Source/WebCore/html/HTMLFormElement.cpp

    r104383 r104668  
    9696
    9797    for (unsigned i = 0; i < m_associatedElements.size(); ++i)
    98         m_associatedElements[i]->formDestroyed();
     98        m_associatedElements[i]->formWillBeDestroyed();
    9999    for (unsigned i = 0; i < m_imageElements.size(); ++i)
    100100        m_imageElements[i]->m_form = 0;
    … …  
    457457void HTMLFormElement::registerFormElement(FormAssociatedElement* e)
    458458{
    459     if (e->isFormControlElement()) {
    460         HTMLFormControlElement* element = static_cast<HTMLFormControlElement*>(e);
    461         document()->checkedRadioButtons().removeButton(element);
    462         m_checkedRadioButtons.addButton(element);
    463     }
    464459    m_associatedElements.insert(formElementIndex(e), e);
    465460}
    … …  
    467462void HTMLFormElement::removeFormElement(FormAssociatedElement* e)
    468463{
    469     if (e->isFormControlElement())
    470         m_checkedRadioButtons.removeButton(static_cast<HTMLFormControlElement*>(e));
    471464    unsigned index;
    472465    for (index = 0; index < m_associatedElements.size(); ++index) {

  • trunk/Source/WebCore/html/HTMLInputElement.cpp

    r104383 r104668  
    117117        document()->unregisterForPageCacheSuspensionCallbacks(this);
    118118
    119     document()->checkedRadioButtons().removeButton(this);
    120 
    121     // Need to remove this from the form while it is still an HTMLInputElement,
    122     // so can't wait for the base class's destructor to do it.
    123     removeFromForm();
     119    // Need to remove form association while this is still an HTMLInputElement
     120    // so that virtual functions are called correctly.
     121    setForm(0);
    124122}
    125123
    … …  
    180178void HTMLInputElement::updateCheckedRadioButtons()
    181179{
    182     if (attached() && checked())
    183         checkedRadioButtons().addButton(this);
     180    checkedRadioButtons().addButton(this);
    184181
    185182    if (form()) {
    … …  
    15031500}
    15041501
     1502void HTMLInputElement::willChangeForm()
     1503{
     1504    checkedRadioButtons().removeButton(this);
     1505    HTMLTextFormControlElement::willChangeForm();
     1506}
     1507
     1508void HTMLInputElement::didChangeForm()
     1509{
     1510    HTMLTextFormControlElement::didChangeForm();
     1511    checkedRadioButtons().addButton(this);
     1512}
     1513
     1514void HTMLInputElement::insertedIntoDocument()
     1515{
     1516    HTMLTextFormControlElement::insertedIntoDocument();
     1517    ASSERT(inDocument());
     1518    checkedRadioButtons().addButton(this);
     1519}
     1520
     1521void HTMLInputElement::removedFromDocument()
     1522{
     1523    ASSERT(inDocument());
     1524    checkedRadioButtons().removeButton(this);
     1525    HTMLTextFormControlElement::removedFromDocument();
     1526}
     1527
    15051528void HTMLInputElement::didMoveToNewDocument(Document* oldDocument)
    15061529{

  • trunk/Source/WebCore/html/HTMLInputElement.h

    r103679 r104668  
    250250    enum AnyStepHandling { RejectAny, AnyIsDefaultStep };
    251251
     252    virtual void willChangeForm() OVERRIDE;
     253    virtual void didChangeForm() OVERRIDE;
     254    virtual void insertedIntoDocument() OVERRIDE;
     255    virtual void removedFromDocument() OVERRIDE;
    252256    virtual void didMoveToNewDocument(Document* oldDocument) OVERRIDE;
    253257

  • trunk/Source/WebCore/html/HTMLObjectElement.cpp

    r103679 r104668  
    5656inline HTMLObjectElement::HTMLObjectElement(const QualifiedName& tagName, Document* document, HTMLFormElement* form, bool createdByParser)
    5757    : HTMLPlugInImageElement(tagName, document, createdByParser, ShouldNotPreferPlugInsForImages)
    58     , FormAssociatedElement(form)
    5958    , m_docNamedItem(true)
    6059    , m_useFallbackContent(false)
    6160{
    6261    ASSERT(hasTagName(objectTag));
    63     if (!this->form())
    64         setForm(findFormAncestor());
    65     if (this->form())
    66         this->form()->registerFormElement(this);
     62    setForm(form ? form : findFormAncestor());
    6763}
    6864
    6965inline HTMLObjectElement::~HTMLObjectElement()
    7066{
    71     if (form())
    72         form()->removeFormElement(this);
    7367}
    7468
Note: See TracChangeset for help on using the changeset viewer.