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: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 84635, 84796, 110952, 227534, 227839 | ||||||||||||
Attachments: |
|
Description
Tim Nguyen (:ntim)
2021-07-08 10:02:50 PDT
Created attachment 433206 [details]
Patch
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.
> for (auto& element : m_topLayerElement) {
> ...
> }
that would be
for (auto& element : WTF::makeReversedRange(m_topLayerElement)) {
...
}
Created attachment 433208 [details]
Patch
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). Created attachment 433209 [details]
Patch
Created attachment 433210 [details]
Patch
Committed r279780 (239547@main): <https://commits.webkit.org/239547@main> |