[webkit-reviews] review denied: [Bug 87614] REGRESSION(r109729): The optgroup element's "disabled" attribute has no effect to rendering and selection : [Attachment 144309] Patch 2

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied yosin at chromium.org's request for
review:
Bug 87614: REGRESSION(r109729): The optgroup element's "disabled" attribute has
no effect to rendering and selection
https://bugs.webkit.org/show_bug.cgi?id=87614

Attachment 144309: Patch 2
https://bugs.webkit.org/attachment.cgi?id=144309&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=144309&action=review


> Source/WebCore/html/HTMLOptGroupElement.cpp:56
> +    if (m_disabled)

m_disabled is not needed. fastHasAttribute(disabledAttr) is enough.

> Source/WebCore/html/HTMLOptGroupElement.cpp:97
> -    HTMLElement::parseAttribute(attribute);
> +    if (attribute.name() == disabledAttr) {
> +	   bool oldDisabled = m_disabled;
> +	   m_disabled = !attribute.isNull();
> +	   if (oldDisabled != m_disabled) {
> +	       setNeedsStyleRecalc();
> +	       if (renderer() && renderer()->style()->hasAppearance())
> +		   renderer()->theme()->stateChanged(renderer(), EnabledState);

> +	   }
> +    } else
> +	   HTMLElement::parseAttribute(attribute);

This change is not needed because <optgroup> never has a renderer, and
appearance update by disabled change is handled in recalcSelectOptions().


> Source/WebCore/html/HTMLOptGroupElement.h:63
> +    bool m_disabled;

m_disabled is not needed.