LibWeb: !is_top_layer() verification failure in show_popover when clicking a link to a popular repository on GitHub organisation page

[Lubrsi]

For example, when clicking on "ladybird" on https://github.com/LadybirdBrowser

VERIFICATION FAILED: !in_top_layer() at /Users/lukewilde/Repositories/ladybird/Libraries/LibWeb/HTML/HTMLElement.cpp:992
0   liblagom-ak.0.0.0.dylib             0x0000000100ebea60 ak_trap + 56
1   liblagom-ak.0.0.0.dylib             0x0000000100ebed74 ak_assertion_failed + 0
2   liblagom-web.0.0.0.dylib            0x0000000102831000 Web::HTML::HTMLElement::show_popover(Web::HTML::ThrowExceptions, GC::Ptr<Web::HTML::HTMLElement>) + 660
3   liblagom-web.0.0.0.dylib            0x0000000102f2438c Web::Bindings::HTMLElementPrototype::show_popover(JS::VM&) + 520
4   liblagom-js.0.0.0.dylib             0x0000000101084a94 AK::Function<JS::ThrowCompletionOr<void> (JS::Declaration const&)>::operator()(JS::Declaration const&) const + 80
5   liblagom-js.0.0.0.dylib             0x000000010124a070 JS::NativeFunction::internal_call(JS::Value, AK::Span<JS::Value const>) + 388
6   liblagom-js.0.0.0.dylib             0x00000001010cd760 JS::Bytecode::Interpreter::run_bytecode(unsigned long) + 8820
7   liblagom-js.0.0.0.dylib             0x00000001010cb214 JS::Bytecode::Interpreter::run_executable(JS::Bytecode::Executable&, AK::Optional<unsigned long>, JS::Value) + 408
8   liblagom-js.0.0.0.dylib             0x00000001011ddb28 JS::GeneratorObject::execute(JS::VM&, JS::Completion const&) + 296
9   liblagom-js.0.0.0.dylib             0x00000001011dde34 JS::GeneratorObject::resume(JS::VM&, JS::Value, AK::Optional<AK::StringView> const&) + 328
10  liblagom-js.0.0.0.dylib             0x0000000101188eb0 JS::AsyncFunctionDriverWrapper::continue_async_execution(JS::VM&, JS::Value, bool, JS::AsyncFunctionDriverWrapper::IsInitialExecution) + 92
11  liblagom-js.0.0.0.dylib             0x000000010118982c AK::Function<JS::ThrowCompletionOr<JS::Value> (JS::VM&)>::CallableWrapper<JS::AsyncFunctionDriverWrapper::await(JS::Value)::$_0>::call(JS::VM&) + 196
12  liblagom-js.0.0.0.dylib             0x0000000101084a94 AK::Function<JS::ThrowCompletionOr<void> (JS::Declaration const&)>::operator()(JS::Declaration const&) const + 80
13  liblagom-js.0.0.0.dylib             0x000000010124a070 JS::NativeFunction::internal_call(JS::Value, AK::Span<JS::Value const>) + 388
14  liblagom-web.0.0.0.dylib            0x0000000102543c54 AK::Function<JS::ThrowCompletionOr<JS::Value> (JS::JobCallback&, JS::Value, AK::Span<JS::Value const>)>::CallableWrapper<Web::Bindings::initialize_main_thread_vm(Web::HTML::EventLoop::Type)::$_5>::call(JS::JobCallback&, JS::Value, AK::Span<JS::Value const>) + 128
15  liblagom-js.0.0.0.dylib             0x0000000101084a94 AK::Function<JS::ThrowCompletionOr<void> (JS::Declaration const&)>::operator()(JS::Declaration const&) const + 80
16  liblagom-js.0.0.0.dylib             0x000000010126ce9c AK::Function<JS::ThrowCompletionOr<JS::Value> ()>::CallableWrapper<JS::create_promise_reaction_job(JS::VM&, JS::PromiseReaction&, JS::Value)::$_0>::call() + 76
17  liblagom-web.0.0.0.dylib            0x0000000102544458 AK::Function<JS::ThrowCompletionOr<void> (AK::String const&, AK::String const&)>::operator()(AK::String const&, AK::String const&) const + 80
18  liblagom-web.0.0.0.dylib            0x000000010254430c AK::Function<void ()>::CallableWrapper<Web::Bindings::initialize_main_thread_vm(Web::HTML::EventLoop::Type)::$_7::operator()(GC::Ref<GC::Function<JS::ThrowCompletionOr<JS::Value> ()>>, JS::Realm*) const::'lambda'()>::call() + 168
19  liblagom-web.0.0.0.dylib            0x000000010251b8d4 AK::Function<void (AK::Span<unsigned char>)>::operator()(AK::Span<unsigned char>) const + 76
20  liblagom-web.0.0.0.dylib            0x000000010280a1d8 Web::HTML::EventLoop::perform_a_microtask_checkpoint() + 56
21  liblagom-web.0.0.0.dylib            0x0000000102acfcdc Web::WebIDL::call_user_object_operation(Web::WebIDL::CallbackType&, AK::String const&, AK::Optional<JS::Value>, GC::MarkedVector<JS::Value, 0ul>) + 480
22  liblagom-web.0.0.0.dylib            0x00000001027418d4 JS::Completion Web::WebIDL::call_user_object_operation<Web::DOM::Node*>(Web::WebIDL::CallbackType&, AK::String const&, AK::Optional<JS::Value>, Web::DOM::Node*&&) + 356
23  liblagom-web.0.0.0.dylib            0x0000000102741698 Web::DOM::EventDispatcher::inner_invoke(Web::DOM::Event&, AK::Vector<GC::Root<Web::DOM::DOMEventListener>, 0ul>&, Web::DOM::Event::Phase, bool, bool&) + 520
24  liblagom-web.0.0.0.dylib            0x0000000102741a20 Web::DOM::EventDispatcher::invoke(Web::DOM::Event::PathEntry&, Web::DOM::Event&, Web::DOM::Event::Phase, bool&) + 196
25  liblagom-web.0.0.0.dylib            0x00000001027423d8 Web::DOM::EventDispatcher::dispatch(GC::Ref<Web::DOM::EventTarget>, Web::DOM::Event&, bool, bool&) + 2008
26  liblagom-web.0.0.0.dylib            0x0000000102741bd8 Web::DOM::EventDispatcher::dispatch(GC::Ref<Web::DOM::EventTarget>, Web::DOM::Event&, bool) + 36
27  liblagom-web.0.0.0.dylib            0x00000001028110a8 Web::HTML::fire_a_focus_event(GC::Ptr<Web::DOM::EventTarget>, GC::Ptr<Web::DOM::EventTarget>, AK::FlyString const&, bool) + 156
28  liblagom-web.0.0.0.dylib            0x0000000102810a50 Web::HTML::run_focus_update_steps(AK::Vector<GC::Root<Web::DOM::Node>, 0ul>, AK::Vector<GC::Root<Web::DOM::Node>, 0ul>, Web::DOM::Node*) + 424
29  liblagom-web.0.0.0.dylib            0x0000000102810568 Web::HTML::run_focusing_steps(Web::DOM::Node*, Web::DOM::Node*, AK::Optional<AK::ByteString>) + 272
30  liblagom-web.0.0.0.dylib            0x00000001029e5024 Web::EventHandler::handle_mousedown(Gfx::Point<Web::CSSPixels>, Gfx::Point<Web::CSSPixels>, unsigned int, unsigned int, unsigned int) + 1220
31  WebContent                          0x000000010064d114 WebContent::ConnectionFromClient::process_next_input_event() + 332
32  liblagom-web.0.0.0.dylib            0x000000010251b8d4 AK::Function<void (AK::Span<unsigned char>)>::operator()(AK::Span<unsigned char>) const + 76
33  liblagom-core.0.0.0.dylib           0x0000000100be3ba4 AK::Function<void (AK::Error&)>::operator()(AK::Error&) const + 76
34  liblagom-core.0.0.0.dylib           0x0000000100be30e4 Core::EventReceiver::dispatch_event(Core::Event&, Core::EventReceiver*) + 112
35  liblagom-core.0.0.0.dylib           0x0000000100bf568c Core::ThreadEventQueue::process() + 400
36  liblagom-core.0.0.0.dylib           0x0000000100bf7924 Core::EventLoopImplementationUnix::exec() + 44
37  liblagom-core.0.0.0.dylib           0x0000000100be0c5c Core::EventLoop::exec() + 72
38  WebContent                          0x0000000100648fa8 serenity_main(Main::Arguments) + 4248
39  WebContent                          0x00000001006e5a2c main + 196
40  dyld                                0x00000001920ec274 start + 2840

[tcl3]

Minimal repro:

<!DOCTYPE html>
<div id="test" popover>Test</div>
<script>
    const popoverElement = document.getElementById("test");
    popoverElement.showPopover();
    popoverElement.hidePopover();
    popoverElement.showPopover();
</script>

The issue seems to be that the call to hidePopover() sets the popover visibility state to hidden, but doesn't immediately remove the element from the document's top layer when fire_events is true.

I'm not sure if this is covered by some spec steps that we don't implement, or if there needs to be some kind of extra check in check_popover_validity.

What do you think @lukewarlow?

[lukewarlow]

I think this should be addressed once more of the steps are implemented. For example I've got follow ups to deal with close watcher and some of the events.