Bug 230845 – Remove Node::deprecatedIsInert

• Component: CSS
• Status: RESOLVED FIXED

[Tim Nguyen (:ntim)]

There's a invalidation bug in the cached computed style preventing the only usage of Node::deprecatedIsInert in Element::isFocusable in favor of RenderStyle::effectiveInert.

[Radar WebKit Bug Importer]

<rdar://problem/83757966>

[Ryosuke Niwa]

yeah, I don't think we should do this. This would require that Element::isFocusable() to rely on the latest style information. That's not tenable given this function needs to be fast and avoid style recalc.

I really don't think we should have designed the concept of inert to rely on style resolution at all.

[Tim Nguyen (:ntim)]

(In reply to Ryosuke Niwa from comment #2)
> yeah, I don't think we should do this. This would require that
> Element::isFocusable() to rely on the latest style information. That's not
> tenable given this function needs to be fast and avoid style recalc.
> 
> I really don't think we should have designed the concept of inert to rely on
> style resolution at all.

The idea is to re-use isVisibleWithoutResolvingFullStyle (used by isFocusable), which does not resolve the full style. The problem is that the style is not invalidated correctly in some cases.

https://webkit-search.igalia.com/webkit/rev/fb57dd12e0db42483fce7f5e75a2e4ab66ae11a9/Source/WebCore/dom/Element.cpp#3553

So we already consult the style right now in Element::isFocusable for display/visibility, this would just add effectiveInert in it. I don't think this would be slower, since the style is already consulted for those 2 properties anyway.

[Tim Nguyen (:ntim)]

I would argue this would be faster, since effectiveInert is just a style bit that can be looked at once we consulted the style for display/visibility, while Node::deprecatedIsInert requires some DOM tree walks.

[Tim Nguyen (:ntim)]

Created attachment 450506 [details]
Patch

[Antti Koivisto]

Comment on attachment 450506 [details]
Patch

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

> LayoutTests/imported/w3c/ChangeLog:9
> +        This change unfortunately regresses focusability state when dynamically setting inert, due to a cached
> +        computed style invalidation bug.

Can you also explain why these regressions are minor in practice?

[Tim Nguyen (:ntim)]

Created attachment 453290 [details]
[fast-cq] Patch

[EWS]

Committed r290554 (247832@main): <https://commits.webkit.org/247832@main>

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

[Tim Nguyen (:ntim)]

This actually fixed bug 237054!

[Tim Nguyen (:ntim)]

*** Bug 237054 has been marked as a duplicate of this bug. ***