Bug 235003 – Use IsNegation bit for more efficient pseudo-class style invalidation

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antti Koivisto]

We already use it for class invalidation.

[Antti Koivisto]

Created attachment 448668 [details]
Patch

[Antti Koivisto]

Created attachment 448669 [details]
Patch

[Antti Koivisto]

Created attachment 448670 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 448670 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        We now know is an invaliation ruleset is for negated context. We can use this to avoid

Does not parse

> Source/WebCore/dom/Element.cpp:799
> +        Style::PseudoClassChangeInvalidation styleInvalidation(*this, CSSSelector::PseudoClassActive, flag, invalidationScope);

flag needs a better name

> Source/WebCore/dom/Element.cpp:851
> +    Style::PseudoClassChangeInvalidation focusStyleInvalidation(*this, { { CSSSelector::PseudoClassFocus, flag }, { CSSSelector::PseudoClassFocusVisible, flag } });

Ditto

> Source/WebCore/dom/Element.cpp:887
> +        Style::PseudoClassChangeInvalidation styleInvalidation(*this, CSSSelector::PseudoClassFocusWithin, flag);

Ditto

> Source/WebCore/dom/Element.cpp:897
> +        Style::PseudoClassChangeInvalidation styleInvalidation(*this, CSSSelector::PseudoClassHover, flag, invalidationScope);

Ditto

> Source/WebCore/dom/Element.cpp:910
> +    Style::PseudoClassChangeInvalidation styleInvalidation(*this, CSSSelector::PseudoClassDrag, flag);

Ditto

> Source/WebCore/html/HTMLInputElement.cpp:978
> -void HTMLInputElement::setChecked(bool nowChecked)
> +void HTMLInputElement::setChecked(bool newChecked)

The rename doesn't really help. why not just isChecked or checked?

[Antti Koivisto]

Created attachment 448696 [details]
Patch for landing

[EWS]

Committed r287818 (245870@main): <https://commits.webkit.org/245870@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448696 [details].

[Radar WebKit Bug Importer]

<rdar://problem/87304694>

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 448696 [details]
Patch for landing

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

> Source/WebCore/dom/Element.cpp:852
> +    document().userActionElements().setFocused(*this, value);

Probably a bug: this is not the value that ends up getting passed to setHasFocusVisible

[Antti Koivisto]

Comment on attachment 448696 [details]
Patch for landing

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

>> Source/WebCore/dom/Element.cpp:852
>> +    document().userActionElements().setFocused(*this, value);
> 
> Probably a bug: this is not the value that ends up getting passed to setHasFocusVisible

True though I think it just causes unnecessary invalidation in some cases (when focus changes but focus-visible stays the same).