Changeset 193840 in webkit


Ignore:
Timestamp:
Dec 9, 2015 10:19:08 AM (8 years ago)
Author:
commit-queue@webkit.org
Message:

form.elements should reflect the element ordering after the HTML tree builder algorithm
​https://bugs.webkit.org/show_bug.cgi?id=148870
rdar://problem/22589879

Patch by Keith Rollin <Keith Rollin> on 2015-12-09
Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

Rebaseline existing test.

  • web-platform-tests/html/semantics/forms/the-form-element/form-elements-nameditem-02-expected.txt:

Source/WebCore:

form.elements should return form-associated elements in tree order.
However, when presented with an HTML fragment like the following,
forms.elements is not built in tree order. Instead, the elements
appear in forms.element in the same order they appear in the HTML --
that is in the same order as they are parsed.

<form id=form>

<table>

<tr>

<td><input type="radio" name="radio1" id="r1" value=1></td>
<td><input type="radio" name="radio2" id="r2" value=2></td>
<input type="radio" name="radio0" id="r0" value=0>

</tr>

</table>

</form>

The reason why elements appear in forms.elements in parse order is
because they register themselves with the designated form when they
are created. At this time, they are not in the DOM tree, so the form
can only assume that the element will be appended to the DOM tree,
with the result that it records the elements in the HTML fragment
above as [r1, r2, r0].

However, it's not always the case that the newly-created element will
be appended to the current tree. In the HTML fragment above, the r0
input element is hoised out of the table element. It ends up being the
preceding sibling of the table element, with the result that the
actual tree-order of the input elements is [r0, r1, r2].

Because the problem is due to registering form-associated elements
with the form *before* the elements are added to the DOM tree, the
solution is to defer that registration until afterwards. With the new
element in the tree, the form can now use its current location in the
tree to correctly place the element in form.elements.

Existing tests now pass:

  • imported/w3c/web-platform-tests/html/semantics/forms/the-form-element/form-elements-nameditem-02-html
  • html/FormAssociatedElement.cpp:

(WebCore::FormAssociatedElement::FormAssociatedElement):
(WebCore::FormAssociatedElement::insertedInto):
(WebCore::FormAssociatedElement::removedFrom):
(WebCore::FormAssociatedElement::formRemovedFromTree):
(WebCore::FormAssociatedElement::formWillBeDestroyed):

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

(WebCore::HTMLFormControlElement::HTMLFormControlElement):

  • html/HTMLImageElement.cpp:

(WebCore::HTMLImageElement::HTMLImageElement):
(WebCore::HTMLImageElement::insertedInto):
(WebCore::HTMLImageElement::removedFrom):

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

(WebCore::HTMLObjectElement::HTMLObjectElement):

Location:
trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r193832 r193840  
     12015-12-09  Keith Rollin  <krollin@apple.com>
     2
     3        form.elements should reflect the element ordering after the HTML tree builder algorithm
     4        https://bugs.webkit.org/show_bug.cgi?id=148870
     5        rdar://problem/22589879
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Rebaseline existing test.
     10
     11        * web-platform-tests/html/semantics/forms/the-form-element/form-elements-nameditem-02-expected.txt:
     12
    1132015-12-09  Xabier Rodriguez Calvar  <calvaris@igalia.com>
    214

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-form-element/form-elements-nameditem-02-expected.txt

    r189476 r193840  
    22       
    33
    4 FAIL form.elements should work correctly in the face of table syntax errors assert_array_equals: property 0, expected Element node <input type="radio" name="radio0" id="r0" value="0"></input> but got Element node <input type="radio" name="radio1" id="r1" value="1"></input>
     4PASS form.elements should work correctly in the face of table syntax errors
    55

  • trunk/Source/WebCore/ChangeLog

    r193836 r193840  
     12015-12-09  Keith Rollin  <krollin@apple.com>
     2
     3        form.elements should reflect the element ordering after the HTML tree builder algorithm
     4        https://bugs.webkit.org/show_bug.cgi?id=148870
     5        rdar://problem/22589879
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        form.elements should return form-associated elements in tree order.
     10        However, when presented with an HTML fragment like the following,
     11        forms.elements is not built in tree order. Instead, the elements
     12        appear in forms.element in the same order they appear in the HTML --
     13        that is in the same order as they are parsed.
     14
     15        <form id=form>
     16            <table>
     17                <tr>
     18                    <td><input type="radio" name="radio1" id="r1" value=1></td>
     19                    <td><input type="radio" name="radio2" id="r2" value=2></td>
     20                    <input type="radio" name="radio0" id="r0" value=0>
     21                </tr>
     22            </table>
     23        </form>
     24
     25        The reason why elements appear in forms.elements in parse order is
     26        because they register themselves with the designated form when they
     27        are created. At this time, they are not in the DOM tree, so the form
     28        can only assume that the element will be appended to the DOM tree,
     29        with the result that it records the elements in the HTML fragment
     30        above as [r1, r2, r0].
     31
     32        However, it's not always the case that the newly-created element will
     33        be appended to the current tree. In the HTML fragment above, the r0
     34        input element is hoised out of the table element. It ends up being the
     35        preceding sibling of the table element, with the result that the
     36        actual tree-order of the input elements is [r0, r1, r2].
     37
     38        Because the problem is due to registering form-associated elements
     39        with the form *before* the elements are added to the DOM tree, the
     40        solution is to defer that registration until afterwards. With the new
     41        element in the tree, the form can now use its current location in the
     42        tree to correctly place the element in form.elements.
     43
     44        Existing tests now pass:
     45        - imported/w3c/web-platform-tests/html/semantics/forms/the-form-element/form-elements-nameditem-02-html
     46
     47        * html/FormAssociatedElement.cpp:
     48        (WebCore::FormAssociatedElement::FormAssociatedElement):
     49        (WebCore::FormAssociatedElement::insertedInto):
     50        (WebCore::FormAssociatedElement::removedFrom):
     51        (WebCore::FormAssociatedElement::formRemovedFromTree):
     52        (WebCore::FormAssociatedElement::formWillBeDestroyed):
     53        * html/FormAssociatedElement.h:
     54        * html/HTMLFormControlElement.cpp:
     55        (WebCore::HTMLFormControlElement::HTMLFormControlElement):
     56        * html/HTMLImageElement.cpp:
     57        (WebCore::HTMLImageElement::HTMLImageElement):
     58        (WebCore::HTMLImageElement::insertedInto):
     59        (WebCore::HTMLImageElement::removedFrom):
     60        * html/HTMLImageElement.h:
     61        * html/HTMLObjectElement.cpp:
     62        (WebCore::HTMLObjectElement::HTMLObjectElement):
     63
    1642015-12-09  Gwang Yoon Hwang  <yoon@igalia.com>
    265

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

    r189469 r193840  
    5050};
    5151
    52 FormAssociatedElement::FormAssociatedElement()
     52FormAssociatedElement::FormAssociatedElement(HTMLFormElement* form)
    5353    : m_form(nullptr)
     54    , m_formSetByParser(form)
    5455{
    5556}
    … …  
    6970void FormAssociatedElement::insertedInto(ContainerNode& insertionPoint)
    7071{
     72    HTMLElement& element = asHTMLElement();
     73    if (m_formSetByParser) {
     74        setForm(m_formSetByParser);
     75        m_formSetByParser = nullptr;
     76    }
     77
    7178    if (!insertionPoint.inDocument())
    7279        return;
    7380
    74     HTMLElement& element = asHTMLElement();
    7581    if (element.fastHasAttribute(formAttr))
    7682        resetFormAttributeTargetObserver();
    … …  
    8591    // Otherwise, null out our form and remove ourselves from the form's list of elements.
    8692    if (m_form && element.highestAncestor() != m_form->highestAncestor())
    87         setForm(0);
     93        setForm(nullptr);
    8894}
    8995
    … …  
    113119    ASSERT(m_form);
    114120    if (asHTMLElement().highestAncestor() != formRoot)
    115         setForm(0);
     121        setForm(nullptr);
    116122}
    117123
    … …  
    143149        return;
    144150    willChangeForm();
    145     m_form = 0;
     151    m_form = nullptr;
    146152    didChangeForm();
    147153}

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

    r176174 r193840  
    8989
    9090protected:
    91     FormAssociatedElement();
     91    FormAssociatedElement(HTMLFormElement*);
    9292
    9393    void insertedInto(ContainerNode&);
    … …  
    117117    std::unique_ptr<FormAttributeTargetObserver> m_formAttributeTargetObserver;
    118118    HTMLFormElement* m_form;
     119    HTMLFormElement* m_formSetByParser;
    119120    String m_customValidationMessage;
    120121};

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

    r192129 r193840  
    5050HTMLFormControlElement::HTMLFormControlElement(const QualifiedName& tagName, Document& document, HTMLFormElement* form)
    5151    : LabelableElement(tagName, document)
     52    , FormAssociatedElement(form)
    5253    , m_disabled(false)
    5354    , m_isReadOnly(false)
    … …  
    6263    , m_hasAutofocused(false)
    6364{
    64     setForm(form);
    6565    setHasCustomStyleResolveCallbacks();
    6666}

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

    r192953 r193840  
    5656    : HTMLElement(tagName, document)
    5757    , m_imageLoader(*this)
    58     , m_form(form)
     58    , m_form(nullptr)
     59    , m_formSetByParser(form)
    5960    , m_compositeOperator(CompositeSourceOver)
    6061    , m_imageDevicePixelRatio(1.0f)
    … …  
    6566    ASSERT(hasTagName(imgTag));
    6667    setHasCustomStyleResolveCallbacks();
    67     if (form)
    68         form->registerImgElement(this);
    6968}
    7069
    … …  
    164163        if (!evaluator.eval(MediaQuerySet::createAllowingDescriptionSyntax(source.media()).ptr()))
    165164            continue;
    166        
     165
    167166        float sourceSize = parseSizesAttribute(source.fastGetAttribute(sizesAttr).string(), document().renderView(), document().frame());
    168167        ImageCandidate candidate = bestFitSourceForImageAttributes(document().deviceScaleFactor(), nullAtom, source.fastGetAttribute(srcsetAttr), sourceSize);
    … …  
    290289Node::InsertionNotificationRequest HTMLImageElement::insertedInto(ContainerNode& insertionPoint)
    291290{
    292     if (!m_form) { // m_form can be non-null if it was set in constructor.
     291    if (m_formSetByParser) {
     292        m_form = m_formSetByParser;
     293        m_formSetByParser = nullptr;
     294    }
     295
     296    if (!m_form)
    293297        m_form = HTMLFormElement::findClosestFormAncestor(*this);
    294         if (m_form)
    295             m_form->registerImgElement(this);
    296     }
     298
     299    if (m_form)
     300        m_form->registerImgElement(this);
    297301
    298302    // Insert needs to complete first, before we start updating the loader. Loader dispatches events which could result
    … …  
    302306    if (insertionPoint.inDocument() && !m_lowercasedUsemap.isNull())
    303307        document().addImageElementByLowercasedUsemap(*m_lowercasedUsemap.impl(), *this);
    304    
     308
    305309    if (is<HTMLPictureElement>(parentNode()))
    306310        selectImageSource();
    307    
     311
    308312    // If we have been inserted from a renderer-less document,
    309313    // our loader may have not fetched the image, so do it now.
    … …  
    322326        document().removeImageElementByLowercasedUsemap(*m_lowercasedUsemap.impl(), *this);
    323327
    324     m_form = 0;
     328    m_form = nullptr;
    325329    HTMLElement::removedFrom(insertionPoint);
    326330}
    … …  
    519523
    520524    const AtomicString& usemap = fastGetAttribute(usemapAttr);
    521    
     525
    522526    // If the usemap attribute starts with '#', it refers to a map element in the document.
    523527    if (usemap.string()[0] == '#')

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

    r192953 r193840  
    127127    HTMLImageLoader m_imageLoader;
    128128    HTMLFormElement* m_form;
     129    HTMLFormElement* m_formSetByParser;
    129130    CompositeOperator m_compositeOperator;
    130131    AtomicString m_bestFitImageURL;

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

    r191904 r193840  
    6666inline HTMLObjectElement::HTMLObjectElement(const QualifiedName& tagName, Document& document, HTMLFormElement* form, bool createdByParser)
    6767    : HTMLPlugInImageElement(tagName, document, createdByParser)
     68    , FormAssociatedElement(form)
    6869    , m_docNamedItem(true)
    6970    , m_useFallbackContent(false)
    7071{
    7172    ASSERT(hasTagName(objectTag));
    72     setForm(form);
    7373}
    7474
Note: See TracChangeset for help on using the changeset viewer.