Bug 229330

Summary: Implement hit testing bits of inert subtrees
Product: WebKit Reporter: Tim Nguyen (:ntim) <ntim>
Component: DOMAssignee: Tim Nguyen (:ntim) <ntim>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cdumez, cfleizach, changseok, cmarcelo, dmazzoni, esprehn+autocc, ews-watchlist, fred.wang, glenn, gyuyoung.kim, hi, jcraig, jdiggs, joepeck, kangil.han, koivisto, kondapallykalyan, pangle, pdr, samuel_white, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 230678, 230681    
Bug Blocks: 84635, 165279, 230686    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Tim Nguyen (:ntim) 2021-08-20 02:30:53 PDT
Depending on whether we want to affect JS APIs or not, we can either:
* Change hit testing code to treat inert nodes as pointer-events: none, like in: https://github.com/nt1m/WebKit/commit/991684e0f3b810e566329e0c031342de613933b0
-> This is close to Firefox's implementation
-> Firefox argues the spec should be this way: https://github.com/whatwg/html/issues/5650

* Treat inert nodes more like isDisabledFormControl() (e.g. having similar checks when triggering events)
-> More similar to Chrome's implementation
-> Closer to latest inert attribute PR by the Chromium team: https://github.com/whatwg/html/pull/4288


In the context of <dialog>, I think both approaches are acceptable wrt. the spec and similarly easy. Hit testing also doesn't matter much generally since everything is behind a backdrop in a modal dialog context. This is more relevant for the inert attribute implementation.
Comment 1 Radar WebKit Bug Importer 2021-08-20 14:39:26 PDT
<rdar://problem/82186260>
Comment 2 Tim Nguyen (:ntim) 2021-08-31 11:49:32 PDT
Created attachment 436919 [details]
Patch
Comment 3 Tim Nguyen (:ntim) 2021-09-20 04:45:18 PDT
Created attachment 438657 [details]
Patch
Comment 4 Antti Koivisto 2021-09-20 05:06:31 PDT
Comment on attachment 438657 [details]
Patch

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

> Source/WebCore/rendering/style/StyleRareInheritedData.h:169
> +    bool isInert;

You should include this into the bitfield with the rest of the booleans.
Comment 5 Tim Nguyen (:ntim) 2021-09-20 05:36:51 PDT
Created attachment 438661 [details]
Patch
Comment 6 Tim Nguyen (:ntim) 2021-09-22 14:40:28 PDT
Created attachment 438978 [details]
Patch
Comment 7 Tim Nguyen (:ntim) 2021-09-23 01:24:32 PDT
Created attachment 439020 [details]
Patch
Comment 8 Tim Nguyen (:ntim) 2021-09-23 01:30:44 PDT
Comment on attachment 439020 [details]
Patch

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

> Source/WebCore/style/StyleSharingResolver.cpp:283
> +            return false;

Note to self, this could use an inert attribute feature flag check (though not sure if that slows down the SharingResolver or not)
Comment 9 Tim Nguyen (:ntim) 2021-09-23 01:30:46 PDT
Comment on attachment 439020 [details]
Patch

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

> Source/WebCore/style/StyleSharingResolver.cpp:283
> +            return false;

Note to self, this could use an inert attribute feature flag check (though not sure if that slows down the SharingResolver or not)
Comment 10 Antti Koivisto 2021-09-23 01:31:52 PDT
Comment on attachment 439020 [details]
Patch

Please split this up to pieces that can be reviewed separately (along the lines of "Add effectiveInert bit to RenderStyle")
Comment 11 Tim Nguyen (:ntim) 2021-09-23 05:06:40 PDT
Created attachment 439039 [details]
Patch
Comment 12 Tim Nguyen (:ntim) 2021-09-23 08:03:55 PDT
Created attachment 439042 [details]
Patch
Comment 13 Antti Koivisto 2021-09-24 02:28:12 PDT
Comment on attachment 439042 [details]
Patch

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

> Source/WebCore/style/StyleAdjuster.cpp:526
> +    bool isInertSubtreeRoot = (m_document.activeModalDialog() && m_element == m_document.documentElement())
> +        || (m_document.settings().inertAttributeEnabled() && is<HTMLElement>(m_element) && m_element->hasAttribute(HTMLNames::inertAttr));

This would read better as a lambda.

> Source/WebCore/style/StyleAdjuster.cpp:532
> +    if (isInertSubtreeRoot)
> +        style.setEffectiveInert(true);
> +
> +    // Make sure the active dialog is interactable when the whole document is blocked by the modal dialog
> +    if (m_element == m_document.activeModalDialog())
> +        style.setEffectiveInert(false);

What should happen when you have an active <dialog inert>?
Comment 14 Tim Nguyen (:ntim) 2021-09-25 06:50:50 PDT
Created attachment 439262 [details]
Patch
Comment 15 Tim Nguyen (:ntim) 2021-09-25 06:53:40 PDT
Committed r283079 (242137@main): <https://commits.webkit.org/242137@main>
Comment 16 Tim Nguyen (:ntim) 2021-10-04 00:39:35 PDT
*** Bug 229410 has been marked as a duplicate of this bug. ***