Bug 231721 - Don't run focusing steps on disconnected or inert <dialog>
Summary: Don't run focusing steps on disconnected or inert <dialog>
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
Depends on:
Blocks: dialog-element 227537
  Show dependency tree / graph
 
Reported: 2021-10-13 22:22 PDT by Tim Nguyen (:ntim)
Modified: 2021-10-14 10:54 PDT (History)
8 users (show)

See Also:

Attachments
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
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

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]
Patch
Comment 2 Simon Fraser (smfr) 2021-10-14 08:44:08 PDT 
Comment on attachment 441210 [details]
Patch

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 
<rdar://problem/84263950>