Bug 227800

Summary:

Add topLayerElements() and activeModalDialog() to Document

Product:

WebKit

Reporter:

Tim Nguyen (:ntim) <ntim>

Component:

DOM

Assignee:

Tim Nguyen (:ntim) <ntim>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, changseok, cmarcelo, esprehn+autocc, ews-watchlist, gyuyoung.kim, kangil.han, koivisto, ntim, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

84635, 84796, 110952, 227534, 227839

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Tim Nguyen (:ntim)

Reported 2021-07-08 10:02:50 PDT

Just add the DOM bits, so they can be used for bug 227534 and bug 110952, before the rendering bits get implemented.

Attachments
Patch (5.70 KB, patch) 2021-07-09 02:46 PDT, Tim Nguyen (:ntim)	no flags	Details Formatted Diff Diff
Patch (5.65 KB, patch) 2021-07-09 04:46 PDT, Tim Nguyen (:ntim)	no flags	Details Formatted Diff Diff
Patch (5.69 KB, patch) 2021-07-09 04:57 PDT, Tim Nguyen (:ntim)	no flags	Details Formatted Diff Diff
Patch (5.67 KB, patch) 2021-07-09 05:01 PDT, Tim Nguyen (:ntim)	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2021-07-08 10:07:13 PDT

<rdar://problem/80330046>

Tim Nguyen (:ntim)

Comment 2 2021-07-09 02:46:59 PDT

Created attachment 433206 [details] Patch

Antti Koivisto

Comment 3 2021-07-09 03:44:54 PDT

Comment on attachment 433206 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433206&action=review > Source/WebCore/dom/Document.cpp:8419 > +void Document::removeFromTopLayer(Element* element) > +{ > + size_t position = m_topLayerElements.find(element); > + if (position != notFound) > + m_topLayerElements.remove(position); > + > + element->invalidateStyle(); > +} With ListHasSet you can just do m_topLayerElements.remove(element). > Source/WebCore/dom/Document.cpp:8426 > + for (auto it = m_topLayerElements.rbegin(); it != m_topLayerElements.rend(); ++it) { > + if (is<HTMLDialogElement>(*it)) > + return downcast<HTMLDialogElement>((*it).get()); > + } for (auto& element : m_topLayerElement) { ... } > Source/WebCore/dom/Document.h:1506 > + void addToTopLayer(Element*); > + void removeFromTopLayer(Element*); These shouldn't be null so please use Element&. > Source/WebCore/dom/Document.h:2198 > + Vector<RefPtr<Element>> m_topLayerElements; You might want ListHashSet for O(1) removals and membership testing (though a reasonable case shouldn't have many of these). > Source/WebCore/html/HTMLDialogElement.cpp:126 > + if (m_isModal) > + document().removeFromTopLayer(this); > + > m_isModal = false; could put m_isModal = false inside the branch as well.

Antti Koivisto

Comment 4 2021-07-09 03:50:29 PDT

> for (auto& element : m_topLayerElement) { > ... > } that would be for (auto& element : WTF::makeReversedRange(m_topLayerElement)) { ... }

Tim Nguyen (:ntim)

Comment 5 2021-07-09 04:46:48 PDT

Created attachment 433208 [details] Patch

Antti Koivisto

Comment 6 2021-07-09 04:52:04 PDT

Comment on attachment 433208 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433208&action=review > Source/WebCore/dom/Document.cpp:8416 > +void Document::removeFromTopLayer(Element& element) > +{ > + m_topLayerElements.remove(&element); > + > + element.invalidateStyle(); > +} You shouldn't invalidate style here unless the element was actually found from m_topLayerElements (this is being called unconditionally from removedFromAncestor).

Tim Nguyen (:ntim)

Comment 7 2021-07-09 04:57:48 PDT

Created attachment 433209 [details] Patch

Tim Nguyen (:ntim)

Comment 8 2021-07-09 05:01:34 PDT

Created attachment 433210 [details] Patch

Tim Nguyen (:ntim)

Comment 9 2021-07-09 05:05:34 PDT

Committed r279780 (239547@main): <https://commits.webkit.org/239547@main>

Note You need to log in before you can comment on or make changes to this bug.