[webkit-reviews] review denied: [Bug 129465] :active style is not cleared when its display property is set to none before mouse released. : [Attachment 225542] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Sanghyup Lee <sh53.lee at samsung.com>'s
request for review:
Bug 129465: :active style is not cleared when its display property is set to
none before mouse released.
https://bugs.webkit.org/show_bug.cgi?id=129465

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=225542&action=review


> Source/WebCore/dom/Document.cpp:5839
> +	   if (!oldActiveElement->renderer())
> +	       oldActiveElement->setActive(false);

This code change seems like only a partial fix. Last time
Document::updateHoverActiveState was called, we called setActive(true) on
multiple elements. It seems that elements that were ancestors of the main
active one will have the same bug as before. We should construct test cases
that cover that and fix the whole problem instead of patching over the most
obvious surface part of the problem.

There’s no real need for the !oldActiveElement->renderer() check. It would be
harmless and clearer to call oldActiveElement->setActive(false)
unconditionally. It’s hard to understand why we would do this only for an
element without a renderer.

Slightly ugly to fetch the renderer twice, here and for the loop just below it.