Bug 23949 – HTMLSelectElement is in inconsistent state when handling mutation events

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Alexey Proskuryakov]

HTMLSelectElement modification methods call setRecalcListItems() after the fact - but mutation event handlers need to see a consistent select state, too. This causes assertion failures.

Patch is forthcoming.

[Alexey Proskuryakov]

Created attachment 27662 [details]
proposed fix

[Darin Adler]

Comment on attachment 27662 [details]
proposed fix

>  bool HTMLOptGroupElement::insertBefore(PassRefPtr<Node> newChild, Node* refChild, ExceptionCode& ec, bool shouldLazyAttach)
>  {
>      bool result = HTMLFormControlElement::insertBefore(newChild, refChild, ec, shouldLazyAttach);
> -    if (result)
> -        recalcSelectOptions();
>      return result;
>  }
>  
>  bool HTMLOptGroupElement::replaceChild(PassRefPtr<Node> newChild, Node* oldChild, ExceptionCode& ec, bool shouldLazyAttach)
>  {
>      bool result = HTMLFormControlElement::replaceChild(newChild, oldChild, ec, shouldLazyAttach);
> -    if (result)
> -        recalcSelectOptions();
>      return result;
>  }
>  
>  bool HTMLOptGroupElement::removeChild(Node* oldChild, ExceptionCode& ec)
>  {
>      bool result = HTMLFormControlElement::removeChild(oldChild, ec);
> -    if (result)
> -        recalcSelectOptions();
>      return result;
>  }
>  
>  bool HTMLOptGroupElement::appendChild(PassRefPtr<Node> newChild, ExceptionCode& ec, bool shouldLazyAttach)
>  {
>      bool result = HTMLFormControlElement::appendChild(newChild, ec, shouldLazyAttach);
> -    if (result)
> -        recalcSelectOptions();
>      return result;
>  }
>  
>  bool HTMLOptGroupElement::removeChildren()
>  {
>      bool result = HTMLFormControlElement::removeChildren();
> -    if (result)
> -        recalcSelectOptions();
>      return result;
>  }

These overrides should all be removed entirely.

> @@ -299,40 +295,30 @@ void HTMLSelectElement::restoreState(con
>  bool HTMLSelectElement::insertBefore(PassRefPtr<Node> newChild, Node* refChild, ExceptionCode& ec, bool shouldLazyAttach)
>  {
>      bool result = HTMLFormControlElementWithState::insertBefore(newChild, refChild, ec, shouldLazyAttach);
> -    if (result)
> -        setRecalcListItems();
>      return result;
>  }
>  
>  bool HTMLSelectElement::replaceChild(PassRefPtr<Node> newChild, Node *oldChild, ExceptionCode& ec, bool shouldLazyAttach)
>  {
>      bool result = HTMLFormControlElementWithState::replaceChild(newChild, oldChild, ec, shouldLazyAttach);
> -    if (result)
> -        setRecalcListItems();
>      return result;
>  }
>  
>  bool HTMLSelectElement::removeChild(Node* oldChild, ExceptionCode& ec)
>  {
>      bool result = HTMLFormControlElementWithState::removeChild(oldChild, ec);
> -    if (result)
> -        setRecalcListItems();
>      return result;
>  }
>  
>  bool HTMLSelectElement::appendChild(PassRefPtr<Node> newChild, ExceptionCode& ec, bool shouldLazyAttach)
>  {
>      bool result = HTMLFormControlElementWithState::appendChild(newChild, ec, shouldLazyAttach);
> -    if (result)
> -        setRecalcListItems();
>      return result;
>  }
>  
>  bool HTMLSelectElement::removeChildren()
>  {
>      bool result = HTMLFormControlElementWithState::removeChildren();
> -    if (result)
> -        setRecalcListItems();
>      return result;
>  }

These too.

I haven't reviewed the rest of the patch yet.

[Alexey Proskuryakov]

> These overrides should all be removed entirely.

I'm actually adding code in these in bug 13287. 

[Alexey Proskuryakov]

Committed revision 41713.