Skip to content

LibWeb: Hide unrelated popovers when showing popovers #3537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tcl3 merged 2 commits into from
Feb 16, 2025
Merged

LibWeb: Hide unrelated popovers when showing popovers #3537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57bace9
LibWeb: Create a BlockContainer for 'block ruby' elements
Gingeh Feb 3, 2025
b93256e
LibWeb: Hide unrelated popovers when showing popovers
Gingeh Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Libraries/LibWeb/DOM/Document.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,8 @@ void Document::visit_edges(Cell::Visitor& visitor)

visitor.visit(m_top_layer_elements);
visitor.visit(m_top_layer_pending_removals);
visitor.visit(m_showing_auto_popover_list);
visitor.visit(m_showing_hint_popover_list);
visitor.visit(m_console_client);
visitor.visit(m_editing_host_manager);
visitor.visit(m_local_storage_holder);
Expand Down
10 changes: 10 additions & 0 deletions Libraries/LibWeb/DOM/Document.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,13 @@ class Document

OrderedHashTable<GC::Ref<Element>> const& top_layer_elements() const { return m_top_layer_elements; }

// AD-HOC: These lists are managed dynamically instead of being generated as needed.
// Spec issue: https://github.com/whatwg/html/issues/11007
Vector<GC::Ref<HTML::HTMLElement>>& showing_auto_popover_list() { return m_showing_auto_popover_list; }
Vector<GC::Ref<HTML::HTMLElement>>& showing_hint_popover_list() { return m_showing_hint_popover_list; }
Vector<GC::Ref<HTML::HTMLElement>> const& showing_auto_popover_list() const { return m_showing_auto_popover_list; }
Vector<GC::Ref<HTML::HTMLElement>> const& showing_hint_popover_list() const { return m_showing_hint_popover_list; }

size_t transition_generation() const { return m_transition_generation; }

// Does document represent an embedded svg img
Expand Down Expand Up @@ -1099,6 +1106,9 @@ class Document
OrderedHashTable<GC::Ref<Element>> m_top_layer_elements;
OrderedHashTable<GC::Ref<Element>> m_top_layer_pending_removals;

Vector<GC::Ref<HTML::HTMLElement>> m_showing_auto_popover_list;
Vector<GC::Ref<HTML::HTMLElement>> m_showing_hint_popover_list;

// https://dom.spec.whatwg.org/#document-allow-declarative-shadow-roots
bool m_allow_declarative_shadow_roots { false };

Expand Down
5 changes: 5 additions & 0 deletions Libraries/LibWeb/DOM/Element.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,11 @@ GC::Ptr<Layout::NodeWithStyle> Element::create_layout_node_for_display_type(DOM:
return document.heap().allocate<Layout::BlockContainer>(document, element, move(style));

dbgln("FIXME: CSS display '{}' not implemented yet.", display.to_string());

// FIXME: We don't actually support `display: block ruby`, this is just a hack to prevent a crash
if (display.is_ruby_inside())
return document.heap().allocate<Layout::BlockContainer>(document, element, move(style));

return document.heap().allocate<Layout::InlineNode>(document, element, move(style));
}

Expand Down
44 changes: 34 additions & 10 deletions Libraries/LibWeb/HTML/HTMLDialogElement.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,23 @@ WebIDL::ExceptionOr<void> HTMLDialogElement::show()
// 9. Set the dialog close watcher with this.
set_close_watcher();
// FIXME: 10. Set this's previously focused element to the focused element.
// FIXME: 11. Let document be this's node document.
// FIXME: 12. Let hideUntil be the result of running topmost popover ancestor given this, document's showing hint popover list, null, and false.
// FIXME: 13. If hideUntil is null, then set hideUntil to the result of running topmost popover ancestor given this, document's showing auto popover list, null, and false.
// FIXME: 14. If hideUntil is null, then set hideUntil to document.
// FIXME: 15. Run hide all popovers until given hideUntil, false, and true.

// 11. Let document be this's node document.
auto document = m_document;

// 12. Let hideUntil be the result of running topmost popover ancestor given this, document's showing hint popover list, null, and false.
Variant<GC::Ptr<HTMLElement>, GC::Ptr<DOM::Document>> hide_until = topmost_popover_ancestor(this, document->showing_hint_popover_list(), nullptr, IsPopover::No);

// 13. If hideUntil is null, then set hideUntil to the result of running topmost popover ancestor given this, document's showing auto popover list, null, and false.
if (!hide_until.get<GC::Ptr<HTMLElement>>())
hide_until = topmost_popover_ancestor(this, document->showing_auto_popover_list(), nullptr, IsPopover::No);

// 14. If hideUntil is null, then set hideUntil to document.
if (!hide_until.get<GC::Ptr<HTMLElement>>())
hide_until = document;

// 15. Run hide all popovers until given hideUntil, false, and true.
hide_all_popovers_until(hide_until, FocusPreviousElement::No, FireEvents::Yes);

// 16. Run the dialog focusing steps given this.
run_dialog_focusing_steps();
Expand Down Expand Up @@ -224,11 +236,23 @@ WebIDL::ExceptionOr<void> HTMLDialogElement::show_a_modal_dialog(HTMLDialogEleme
subject.set_close_watcher();

// FIXME: 18. Set subject's previously focused element to the focused element.
Comment thread
Gingeh marked this conversation as resolved.
// FIXME: 19. Let document be subject's node document.
// FIXME: 20. Let hideUntil be the result of running topmost popover ancestor given subject, document's showing hint popover list, null, and false.
// FIXME: 21. If hideUntil is null, then set hideUntil to the result of running topmost popover ancestor given subject, document's showing auto popover list, null, and false.
// FIXME: 22. If hideUntil is null, then set hideUntil to document.
// FIXME: 23. Run hide all popovers until given hideUntil, false, and true.

// 19. Let document be subject's node document.
auto& document = subject.document();

// 20. Let hideUntil be the result of running topmost popover ancestor given subject, document's showing hint popover list, null, and false.
Variant<GC::Ptr<HTMLElement>, GC::Ptr<DOM::Document>> hide_until = topmost_popover_ancestor(subject, document.showing_hint_popover_list(), nullptr, IsPopover::No);

// 21. If hideUntil is null, then set hideUntil to the result of running topmost popover ancestor given subject, document's showing auto popover list, null, and false.
if (!hide_until.get<GC::Ptr<HTMLElement>>())
hide_until = topmost_popover_ancestor(subject, document.showing_auto_popover_list(), nullptr, IsPopover::No);

// 22. If hideUntil is null, then set hideUntil to document.
if (!hide_until.get<GC::Ptr<HTMLElement>>())
hide_until = GC::Ptr(document);

// 23. Run hide all popovers until given hideUntil, false, and true.
hide_all_popovers_until(hide_until, FocusPreviousElement::No, FireEvents::Yes);

// 24. Run the dialog focusing steps given subject.
subject.run_dialog_focusing_steps();
Expand Down
Loading