form.elements should reflect the element ordering after the HTML tree builder algorithm
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Dec 2015 18:19:08 +0000 (18:19 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Dec 2015 18:19:08 +0000 (18:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148870
rdar://problem/22589879

Patch by Keith Rollin <krollin@apple.com> 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):

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

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-form-element/form-elements-nameditem-02-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/html/FormAssociatedElement.cpp
Source/WebCore/html/FormAssociatedElement.h
Source/WebCore/html/HTMLFormControlElement.cpp
Source/WebCore/html/HTMLImageElement.cpp
Source/WebCore/html/HTMLImageElement.h
Source/WebCore/html/HTMLObjectElement.cpp

index 507daad..382fb22 100644 (file)
@@ -1,3 +1,15 @@
+2015-12-09  Keith Rollin  <krollin@apple.com>
+
+        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
+
+        Reviewed by Ryosuke Niwa.
+
+        Rebaseline existing test.
+
+        * web-platform-tests/html/semantics/forms/the-form-element/form-elements-nameditem-02-expected.txt:
+
 2015-12-09  Xabier Rodriguez Calvar  <calvaris@igalia.com>
 
         [Streams API] pipeThrough test failing
index 9aa3790..09e8fc8 100644 (file)
@@ -1,5 +1,5 @@
 
        
 
-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>
+PASS form.elements should work correctly in the face of table syntax errors 
 
index 1aab7c6..d08fbd7 100644 (file)
@@ -1,3 +1,66 @@
+2015-12-09  Keith Rollin  <krollin@apple.com>
+
+        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
+
+        Reviewed by Ryosuke Niwa.
+
+        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):
+
 2015-12-09  Gwang Yoon Hwang  <yoon@igalia.com>
 
         [ThreadedCompositor] Support HTML5 Video
index 474fb65..7593904 100644 (file)
@@ -49,8 +49,9 @@ private:
     FormAssociatedElement& m_element;
 };
 
-FormAssociatedElement::FormAssociatedElement()
+FormAssociatedElement::FormAssociatedElement(HTMLFormElement* form)
     : m_form(nullptr)
+    , m_formSetByParser(form)
 {
 }
 
@@ -68,10 +69,15 @@ void FormAssociatedElement::didMoveToNewDocument(Document* oldDocument)
 
 void FormAssociatedElement::insertedInto(ContainerNode& insertionPoint)
 {
+    HTMLElement& element = asHTMLElement();
+    if (m_formSetByParser) {
+        setForm(m_formSetByParser);
+        m_formSetByParser = nullptr;
+    }
+
     if (!insertionPoint.inDocument())
         return;
 
-    HTMLElement& element = asHTMLElement();
     if (element.fastHasAttribute(formAttr))
         resetFormAttributeTargetObserver();
 }
@@ -84,7 +90,7 @@ void FormAssociatedElement::removedFrom(ContainerNode& insertionPoint)
     // If the form and element are both in the same tree, preserve the connection to the form.
     // Otherwise, null out our form and remove ourselves from the form's list of elements.
     if (m_form && element.highestAncestor() != m_form->highestAncestor())
-        setForm(0);
+        setForm(nullptr);
 }
 
 HTMLFormElement* FormAssociatedElement::findAssociatedForm(const HTMLElement* element, HTMLFormElement* currentAssociatedForm)
@@ -112,7 +118,7 @@ void FormAssociatedElement::formRemovedFromTree(const Node* formRoot)
 {
     ASSERT(m_form);
     if (asHTMLElement().highestAncestor() != formRoot)
-        setForm(0);
+        setForm(nullptr);
 }
 
 void FormAssociatedElement::setForm(HTMLFormElement* newForm)
@@ -142,7 +148,7 @@ void FormAssociatedElement::formWillBeDestroyed()
     if (!m_form)
         return;
     willChangeForm();
-    m_form = 0;
+    m_form = nullptr;
     didChangeForm();
 }
 
index 7df6c39..f39acf2 100644 (file)
@@ -88,7 +88,7 @@ public:
     void formAttributeTargetChanged();
 
 protected:
-    FormAssociatedElement();
+    FormAssociatedElement(HTMLFormElement*);
 
     void insertedInto(ContainerNode&);
     void removedFrom(ContainerNode&);
@@ -116,6 +116,7 @@ private:
 
     std::unique_ptr<FormAttributeTargetObserver> m_formAttributeTargetObserver;
     HTMLFormElement* m_form;
+    HTMLFormElement* m_formSetByParser;
     String m_customValidationMessage;
 };
 
index 291e1eb..873e375 100644 (file)
@@ -49,6 +49,7 @@ using namespace HTMLNames;
 
 HTMLFormControlElement::HTMLFormControlElement(const QualifiedName& tagName, Document& document, HTMLFormElement* form)
     : LabelableElement(tagName, document)
+    , FormAssociatedElement(form)
     , m_disabled(false)
     , m_isReadOnly(false)
     , m_isRequired(false)
@@ -61,7 +62,6 @@ HTMLFormControlElement::HTMLFormControlElement(const QualifiedName& tagName, Doc
     , m_wasChangedSinceLastFormControlChangeEvent(false)
     , m_hasAutofocused(false)
 {
-    setForm(form);
     setHasCustomStyleResolveCallbacks();
 }
 
index a9424b3..d102339 100644 (file)
@@ -55,7 +55,8 @@ using namespace HTMLNames;
 HTMLImageElement::HTMLImageElement(const QualifiedName& tagName, Document& document, HTMLFormElement* form)
     : HTMLElement(tagName, document)
     , m_imageLoader(*this)
-    , m_form(form)
+    , m_form(nullptr)
+    , m_formSetByParser(form)
     , m_compositeOperator(CompositeSourceOver)
     , m_imageDevicePixelRatio(1.0f)
 #if ENABLE(SERVICE_CONTROLS)
@@ -64,8 +65,6 @@ HTMLImageElement::HTMLImageElement(const QualifiedName& tagName, Document& docum
 {
     ASSERT(hasTagName(imgTag));
     setHasCustomStyleResolveCallbacks();
-    if (form)
-        form->registerImgElement(this);
 }
 
 Ref<HTMLImageElement> HTMLImageElement::create(Document& document)
@@ -163,7 +162,7 @@ ImageCandidate HTMLImageElement::bestFitSourceFromPictureElement()
         MediaQueryEvaluator evaluator(document().printing() ? "print" : "screen", document().frame(), computedStyle());
         if (!evaluator.eval(MediaQuerySet::createAllowingDescriptionSyntax(source.media()).ptr()))
             continue;
-        
+
         float sourceSize = parseSizesAttribute(source.fastGetAttribute(sizesAttr).string(), document().renderView(), document().frame());
         ImageCandidate candidate = bestFitSourceForImageAttributes(document().deviceScaleFactor(), nullAtom, source.fastGetAttribute(srcsetAttr), sourceSize);
         if (!candidate.isEmpty())
@@ -289,22 +288,27 @@ void HTMLImageElement::didAttachRenderers()
 
 Node::InsertionNotificationRequest HTMLImageElement::insertedInto(ContainerNode& insertionPoint)
 {
-    if (!m_form) { // m_form can be non-null if it was set in constructor.
-        m_form = HTMLFormElement::findClosestFormAncestor(*this);
-        if (m_form)
-            m_form->registerImgElement(this);
+    if (m_formSetByParser) {
+        m_form = m_formSetByParser;
+        m_formSetByParser = nullptr;
     }
 
+    if (!m_form)
+        m_form = HTMLFormElement::findClosestFormAncestor(*this);
+
+    if (m_form)
+        m_form->registerImgElement(this);
+
     // Insert needs to complete first, before we start updating the loader. Loader dispatches events which could result
     // in callbacks back to this node.
     Node::InsertionNotificationRequest insertNotificationRequest = HTMLElement::insertedInto(insertionPoint);
 
     if (insertionPoint.inDocument() && !m_lowercasedUsemap.isNull())
         document().addImageElementByLowercasedUsemap(*m_lowercasedUsemap.impl(), *this);
-    
+
     if (is<HTMLPictureElement>(parentNode()))
         selectImageSource();
-    
+
     // If we have been inserted from a renderer-less document,
     // our loader may have not fetched the image, so do it now.
     if (insertionPoint.inDocument() && !m_imageLoader.image())
@@ -321,7 +325,7 @@ void HTMLImageElement::removedFrom(ContainerNode& insertionPoint)
     if (insertionPoint.inDocument() && !m_lowercasedUsemap.isNull())
         document().removeImageElementByLowercasedUsemap(*m_lowercasedUsemap.impl(), *this);
 
-    m_form = 0;
+    m_form = nullptr;
     HTMLElement::removedFrom(insertionPoint);
 }
 
@@ -518,7 +522,7 @@ bool HTMLImageElement::isServerMap() const
         return false;
 
     const AtomicString& usemap = fastGetAttribute(usemapAttr);
-    
+
     // If the usemap attribute starts with '#', it refers to a map element in the document.
     if (usemap.string()[0] == '#')
         return false;
index 81745d2..199e8c0 100644 (file)
@@ -126,6 +126,7 @@ private:
 
     HTMLImageLoader m_imageLoader;
     HTMLFormElement* m_form;
+    HTMLFormElement* m_formSetByParser;
     CompositeOperator m_compositeOperator;
     AtomicString m_bestFitImageURL;
     AtomicString m_currentSrc;
index dd3ca34..aba001b 100644 (file)
@@ -65,11 +65,11 @@ using namespace HTMLNames;
 
 inline HTMLObjectElement::HTMLObjectElement(const QualifiedName& tagName, Document& document, HTMLFormElement* form, bool createdByParser)
     : HTMLPlugInImageElement(tagName, document, createdByParser)
+    , FormAssociatedElement(form)
     , m_docNamedItem(true)
     , m_useFallbackContent(false)
 {
     ASSERT(hasTagName(objectTag));
-    setForm(form);
 }
 
 inline HTMLObjectElement::~HTMLObjectElement()