Bug 229330 - Implement hit testing bits of inert subtrees
Summary: Implement hit testing bits of inert subtrees
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Nguyen (:ntim)
URL:
Keywords: InRadar
: 229410 (view as bug list)
Depends on: 230678 230681
Blocks: dialog-element 165279 230686
  Show dependency treegraph
 
Reported: 2021-08-20 02:30 PDT by Tim Nguyen (:ntim)
Modified: 2021-10-04 00:39 PDT (History)
25 users (show)

See Also:


Attachments
Patch (14.09 KB, patch)
2021-08-31 11:49 PDT, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff
Patch (23.75 KB, patch)
2021-09-20 04:45 PDT, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff
Patch (23.75 KB, patch)
2021-09-20 05:36 PDT, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff
Patch (43.43 KB, patch)
2021-09-22 14:40 PDT, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff
Patch (49.46 KB, patch)
2021-09-23 01:24 PDT, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff
Patch (42.79 KB, patch)
2021-09-23 05:06 PDT, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff
Patch (30.43 KB, patch)
2021-09-23 08:03 PDT, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff
Patch (30.75 KB, patch)
2021-09-25 06:50 PDT, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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. ***