This bug is very similar to bug 13701. The patch for that bug left out textarea and select elements. I am going to attach a patch that fixes that and includes a test for both bugs.
Created attachment 15461 [details] [DO NOT LAND YET] Unregister and reregister when moving from one document to another Another way to do this is to implement HTMLGenericFormElement::{will,did}MoveToNewOwnerDocument, but I am afraid that that will require either a registersWithDocument() function like I had in the first patch for bug 14400 or for HTMLInputElement::didMove... not to call the base class implementation. What do you think?
I'm okay with this patch - but I just had a discussion with mitz on IRC. I think now that we have these new cases that have to patch in to the will/did move document calls that the original patch in 14400 is more appropriate. It would be very easy to adapt here. I look at it this way. If we land this new patch, we'll be maintaining 3 distinct implementations of the will/did move document calls. One in TextArea, one in Select, and one in Input. However, they are largely duplicates of each other. If we go back to Mitz's original patch in 14400, there will be yet another new call to override in genericform and inputelement, and we'll have to maintain it in these multiple places, but it will be much more targeted code will be duplicated. Mitz said he was starting to lean towards that solution as well, but I'm only slightly in favor of reverting back to the original 14400 patch =D
"but it will be much more targeted code will be duplicated." should read "but it will be much more targeted code that makes sense, and not as much code will be duplicated"
Comment on attachment 15461 [details] [DO NOT LAND YET] Unregister and reregister when moving from one document to another This looks like a fine fix to the bug in any case. r=me
Created attachment 15474 [details] Unregister and reregister when moving from one document to another, using a registersWithDocument predicate
Since there are tons of bits available in HTMLGenericFormElement, I grabbed a bit and have what I think is an even better fix.
Created attachment 15540 [details] unregister/register when moving between documents with class to factor out state logic
Comment on attachment 15540 [details] unregister/register when moving between documents with class to factor out state logic Cool! This is the "third option" that I told Brady is was considering. + (WebCore::HTMLSelectElement::~HTMLSelectElement): Removed the call to + unregisterFormElementWithState. HTMLSelectElement::~HTMLSelectElement() { - document()->unregisterFormElementWithState(this); } I don't understand why you left the empty destructor in this case but not in the text area case. r=me!
(In reply to comment #8) > I don't understand why you left the empty destructor in this case but not in > the text area case. Because HTMLSelectElement had members with non-trivial destructors I decided not to rely on the compiler-generated destructor. Probably not a good reason.
Sending LayoutTests/ChangeLog Adding LayoutTests/fast/forms/saved-state-adoptNode-crash-expected.txt Adding LayoutTests/fast/forms/saved-state-adoptNode-crash.html Sending WebCore/ChangeLog Sending WebCore/dom/Document.cpp Sending WebCore/dom/Document.h Sending WebCore/html/HTMLGenericFormElement.cpp Sending WebCore/html/HTMLGenericFormElement.h Sending WebCore/html/HTMLInputElement.cpp Sending WebCore/html/HTMLInputElement.h Sending WebCore/html/HTMLSelectElement.cpp Sending WebCore/html/HTMLSelectElement.h Sending WebCore/html/HTMLTextAreaElement.cpp Sending WebCore/html/HTMLTextAreaElement.h Transmitting file data .............. Committed revision 24363.