RESOLVED FIXED 81420
Break the association between form controls and their owner when the owner leaves the tree
https://bugs.webkit.org/show_bug.cgi?id=81420
Summary Break the association between form controls and their owner when the owner le...
Adam Klein
Reported 2012-03-16 15:24:53 PDT
Break the association between form controls and their owner when the owner leaves the tree
Attachments
Patch (9.53 KB, patch)
2012-03-16 15:34 PDT, Adam Klein
no flags
Added expectation (10.50 KB, patch)
2012-03-16 15:51 PDT, Adam Klein
no flags
Patch (7.23 KB, patch)
2012-04-10 12:20 PDT, Adam Klein
no flags
Adam Klein
Comment 1 2012-03-16 15:34:26 PDT
Adam Klein
Comment 2 2012-03-16 15:51:23 PDT
Created attachment 132409 [details] Added expectation
Kent Tamura
Comment 3 2012-03-20 21:34:51 PDT
Comment on attachment 132409 [details] Added expectation View in context: https://bugs.webkit.org/attachment.cgi?id=132409&action=review > Source/WebCore/html/HTMLFormElement.cpp:161 > + for (unsigned i = 0; i < m_associatedElements.size(); ++i) > + m_associatedElements[i]->formRemovedFromTree(root); This is a bad iteration pattern. formRemovedFromTree() updates m_associatedElements. Form-association code is complex, and introducing FormAssociatedElement::m_formNeedsSameRootCheck makes it more complex. I'd like to avoid m_formNeedsRootCheck as possible. I'd like to change formRemovedFromTree() so that it unregister a control only if the control has form= attribute or its root is different from the root of this form.
Kent Tamura
Comment 4 2012-03-20 21:47:10 PDT
(In reply to comment #3) > I'd like to change formRemovedFromTree() so that it unregister a control only if the control has form= attribute or its root is different from the root of this form. and it does nothing otherwise. I don't think findRoot cost is significant.
Adam Klein
Comment 5 2012-04-10 12:20:18 PDT
Adam Klein
Comment 6 2012-04-10 12:21:43 PDT
Comment on attachment 132409 [details] Added expectation View in context: https://bugs.webkit.org/attachment.cgi?id=132409&action=review >> Source/WebCore/html/HTMLFormElement.cpp:161 >> + m_associatedElements[i]->formRemovedFromTree(root); > > This is a bad iteration pattern. formRemovedFromTree() updates m_associatedElements. > > Form-association code is complex, and introducing FormAssociatedElement::m_formNeedsSameRootCheck makes it more complex. I'd like to avoid m_formNeedsRootCheck as possible. > I'd like to change formRemovedFromTree() so that it unregister a control only if the control has form= attribute or its root is different from the root of this form. Changed to make a copy of the vector before iterating. I've removed m_formNeedsSameRootCheck, greatly simplifying this patch. Note that a form with an ID going away is already handled in HTMLFormElement::removedFromDocument.
Kent Tamura
Comment 7 2012-04-10 18:36:39 PDT
Comment on attachment 136504 [details] Patch ok
WebKit Review Bot
Comment 8 2012-04-10 19:58:06 PDT
Comment on attachment 136504 [details] Patch Clearing flags on attachment: 136504 Committed r113817: <http://trac.webkit.org/changeset/113817>
WebKit Review Bot
Comment 9 2012-04-10 19:58:11 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.