[webkit-reviews] review denied: [Bug 55716] REGRESSION (r76826): HTMLOptionElement causes bogus repaints : [Attachment 84972] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Rob Buis
<rwlbuis at gmail.com>'s request for review:
Bug 55716: REGRESSION (r76826): HTMLOptionElement causes bogus repaints
https://bugs.webkit.org/show_bug.cgi?id=55716

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=84972&action=review

Needs a test that tests repaint with popup and list-style select elements.

> Source/WebCore/html/HTMLOptionElement.cpp:224
>  void HTMLOptionElement::setRenderStyle(PassRefPtr<RenderStyle> newStyle)
>  {
>      m_style = newStyle;
> -    if (HTMLSelectElement* select = ownerSelectElement())
> -	   if (RenderObject* renderer = select->renderer())
> -	       renderer->repaint();
>  }

Can't you remove this whole method?

> Source/WebCore/rendering/RenderListBox.cpp:468
> +void RenderListBox::styleDidChange(StyleDifference diff, const RenderStyle*
oldStyle)
> +{
> +    RenderBlock::styleDidChange(diff, oldStyle);
> +    repaint();
> +}

How is this equivalent to detecting a style change on an individual <option>?

Unconditionally repainting doesn't seem right either.