[webkit-reviews] review denied: [Bug 233688] [selectors] :focus-visible should stop matching after blur : [Attachment 445577] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has denied Manuel Rego Casasnovas
<rego at igalia.com>'s request for review:
Bug 233688: [selectors] :focus-visible should stop matching after blur
https://bugs.webkit.org/show_bug.cgi?id=233688

Attachment 445577: Patch

https://bugs.webkit.org/attachment.cgi?id=445577&action=review




--- Comment #9 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 445577
  --> https://bugs.webkit.org/attachment.cgi?id=445577
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=445577&action=review

> Source/WebCore/dom/Element.cpp:855
> -    Style::PseudoClassChangeInvalidation styleInvalidation(*this,
CSSSelector::PseudoClassFocusVisible);
> +    if (invalidation == Invalidation::PseudoClass)
> +	   Style::PseudoClassChangeInvalidation styleInvalidation(*this,
CSSSelector::PseudoClassFocusVisible);
>      document().userActionElements().setHasFocusVisible(*this, flag);

This won't work, PseudoClassChangeInvalidation needs to be scoped around the
the mutation (setting focus visible flag) as invalidation needs to run in both
the tree before and after the mutation (from constructor and destructor).
Surprised this doesn't break tests.

I don't think you need to do this change though. The only change that is needed
is to call setHasFocusVisible within the scope of focusStyleInvalidation.