Another issue reduced from trenitalia.com. theSelect.insertBefore(new Option("SUCCESS", "SUCCESS", false, true), theSelect.firstChild); theSelect.options[0].selected = true; // FAILS Surprisingly, this is not a regression.
Created attachment 13968 [details] test case
These OPTION elements are created selected, but WebKit doesn't let SELECT know that when inserting them. So, OPTION elements think that they are selected, and ignore setting .selected to true.
Created attachment 13971 [details] extended test case
Created attachment 21176 [details] proposed fix
Comment on attachment 21176 [details] proposed fix + if (newChildPtr->isElementNode() && static_cast<Element*>(newChildPtr)->hasLocalName(optionTag) && !m_multiple) { As far as I can tell, nothing guarantees that the node pointed to by newChild won't be deallocated by this point. You will need to use a RefPtr here. Maybe the right thing to do is to use the "prp" pattern (naming the argument prpNewChild and the local RefPtr newChild). Ideally we'd make a test case that uses DOM mutation events to make this unlikely even occur. Isn't there a better hook we can use to deal with the change in children? At the very least I'd like to see the repeated code shared in a single function rather than pasted three times. Patch otherwise looks good.
Created attachment 28692 [details] proposed fix
Comment on attachment 28692 [details] proposed fix > + (WebCore::dispatchChildInsertionEvents): Increment DOM tree version. This will happen when > + dispatching DOMSubtreeModified again, but the version should be incremented for event > + listeners to have an up to date view of the DOM. > + (WebCore::dispatchChildRemovalEvents): Ditto. Seems too bad that we later increment the DOM tree version a second time unnecessarily. But no big problem. > + (WebCore::HTMLSelectElement::add): This function doesn't look at exception code, so it > + doesn't need to reset it. This sort of change can be risky; the function can accidentally depend on the passed-in exception code being zero. Accordingly, I wish you'd make this change that's not necessarily tied to the rest of the patch in a separate check-in to possibly help people later who might be tracking down a regression. r=me
Committed revision 41772. Will land the "ec = 0" change next.