Bug 231721 - Don't run focusing steps on disconnected or inert <dialog>
Summary: Don't run focusing steps on disconnected or inert <dialog>
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Nguyen (:ntim)
Keywords: InRadar
Depends on:
Blocks: dialog-element 227537
  Show dependency treegraph
Reported: 2021-10-13 22:22 PDT by Tim Nguyen (:ntim)
Modified: 2021-10-14 10:54 PDT (History)
8 users (show)

See Also:

Patch (4.43 KB, patch)
2021-10-14 06:50 PDT, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff
updateStyleIfNeeded patch (4.43 KB, patch)
2021-10-14 09:33 PDT, Tim Nguyen (:ntim)
koivisto: review+
Details | Formatted Diff | Diff
updateLayout patch (4.45 KB, patch)
2021-10-14 09:46 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-10-13 22:22:30 PDT
For show, we need to update both style & layout.

For showModal, we need to update only layout (addToTopLayer called beforehand updates style).
Comment 1 Tim Nguyen (:ntim) 2021-10-14 06:50:02 PDT
Created attachment 441210 [details]
Comment 2 Simon Fraser (smfr) 2021-10-14 08:44:08 PDT
Comment on attachment 441210 [details]

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

> Source/WebCore/html/HTMLDialogElement.cpp:125
> +    if (auto* cs = computedStyle(); cs && cs->effectiveInert())

We don't use abbreviations like "cs".

Why go through computedStyle() and not the Renderer? Have we updated style here yet after opening the dialog?
Comment 3 Tim Nguyen (:ntim) 2021-10-14 09:33:18 PDT
Created attachment 441227 [details]
updateStyleIfNeeded patch
Comment 4 Tim Nguyen (:ntim) 2021-10-14 09:46:11 PDT
Created attachment 441228 [details]
updateLayout patch
Comment 5 Tim Nguyen (:ntim) 2021-10-14 10:38:31 PDT
Comment on attachment 441228 [details]
updateLayout patch

As Antti points out, Element::isFocusable is able to resolve without layout being up-to-date. So the updateLayout patch is not needed.
Comment 6 Tim Nguyen (:ntim) 2021-10-14 10:53:15 PDT
Committed r284174 (242991@main): <https://commits.webkit.org/242991@main>
Comment 7 Radar WebKit Bug Importer 2021-10-14 10:54:16 PDT