WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Added expectation
(10.50 KB, patch)
2012-03-16 15:51 PDT
,
Adam Klein
no flags
Details
Formatted Diff
Diff
Patch
(7.23 KB, patch)
2012-04-10 12:20 PDT
,
Adam Klein
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Adam Klein
Comment 1
2012-03-16 15:34:26 PDT
Created
attachment 132402
[details]
Patch
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
Created
attachment 136504
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug