diff --git a/Libraries/LibWeb/DOM/Document.cpp b/Libraries/LibWeb/DOM/Document.cpp index c2edf7e473..aa0f881227 100644 --- a/Libraries/LibWeb/DOM/Document.cpp +++ b/Libraries/LibWeb/DOM/Document.cpp @@ -6675,14 +6675,16 @@ void Document::update_for_history_step_application(NonnullRefPtrnavigable()) navigable->restore_persisted_state_from_session_history_entry(*entry); // 3. Initialize the navigation API entries for a new document given navigation, entriesForNavigationAPI, and entry. - navigation->initialize_the_navigation_api_entries_for_a_new_document(*entries_for_navigation_api, entry); + if (update_navigation_api) + navigation->initialize_the_navigation_api_entries_for_a_new_document( + *entries_for_navigation_api, entry); } } diff --git a/Libraries/LibWeb/HTML/Navigable.cpp b/Libraries/LibWeb/HTML/Navigable.cpp index 7efafd7491..2718a7f115 100644 --- a/Libraries/LibWeb/HTML/Navigable.cpp +++ b/Libraries/LibWeb/HTML/Navigable.cpp @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -283,6 +284,49 @@ static Vector>* get_session_history_entries_i return nullptr; } +Vector>* append_nested_history_for_child_navigable( + Navigable& parent_navigable, Navigable& child_navigable, SessionHistoryEntry& history_entry) +{ + VERIFY(child_navigable.parent() == &parent_navigable); + + auto parent_doc_state = parent_navigable.active_session_history_entry()->document_state(); + auto& parent_navigable_entries = parent_navigable.get_session_history_entries(); + auto target_step_entry_iterator = parent_navigable_entries.find_if([parent_doc_state](auto& entry) { + return entry->document_state() == parent_doc_state; + }); + if (target_step_entry_iterator == parent_navigable_entries.end()) + return nullptr; + + history_entry.set_step((*target_step_entry_iterator)->step()); + + DocumentState::NestedHistory nested_history { + .id = child_navigable.id(), + .entries { history_entry }, + }; + parent_doc_state->nested_histories().append(move(nested_history)); + return &parent_doc_state->nested_histories().last().entries; +} + +static Vector>* +recreate_missing_nested_history_for_live_child_navigable(TraversableNavigable& traversable, Navigable& navigable) +{ + VERIFY(&navigable != &traversable); + + auto parent = navigable.parent(); + if (!parent) + return nullptr; + + auto container = navigable.container(); + if (!container || container->content_navigable() != &navigable) + return nullptr; + + auto history_entry = navigable.active_session_history_entry(); + if (!history_entry) + return nullptr; + + return append_nested_history_for_child_navigable(*parent, navigable, *history_entry); +} + // https://html.spec.whatwg.org/multipage/document-sequences.html#child-navigable Vector> Navigable::child_navigables() const { @@ -480,11 +524,9 @@ void Navigable::initialize_navigable(NonnullRefPtr document_state } // https://html.spec.whatwg.org/multipage/browsing-the-web.html#getting-the-target-history-entry -RefPtr Navigable::get_the_target_history_entry(int target_step) const +static RefPtr get_the_target_history_entry_from_entries( + Vector> const& entries, int target_step) { - // 1. Let entries be the result of getting session history entries for navigable. - auto& entries = get_session_history_entries(); - // 2. Return the item in entries that has the greatest step less than or equal to step. RefPtr result = nullptr; for (auto& entry : entries) { @@ -501,6 +543,32 @@ RefPtr Navigable::get_the_target_history_entry(int target_s return result; } +RefPtr Navigable::get_the_target_history_entry(int target_step) const +{ + // 1. Let entries be the result of getting session history entries for navigable. + auto& entries = get_session_history_entries(); + + return get_the_target_history_entry_from_entries(entries, target_step); +} + +RefPtr Navigable::get_the_target_history_entry_if_present(int target_step) const +{ + auto traversable = traversable_navigable(); + Vector>* entries = nullptr; + if (this == traversable.ptr()) + entries = &traversable->session_history_entries(); + else + entries = get_session_history_entries_if_present(*traversable, *this); + + // AD-HOC: The spec asserts that a nested history list is found. During queued navigable creation/destruction + // bookkeeping, engines can still observe a child navigable after its iframe has been removed from the + // parent's nested histories. In that case, the detached child has no observable session history effect. + if (!entries) + return nullptr; + + return get_the_target_history_entry_from_entries(*entries, target_step); +} + // https://html.spec.whatwg.org/multipage/browsing-the-web.html#activate-history-entry void Navigable::activate_history_entry(RefPtr entry, GC::Ref document) { @@ -737,13 +805,10 @@ void Navigable::set_ongoing_navigation(Variant ongoing } // AD-HOC: If we just finished a traversal and there are navigations that were deferred because the traversal was - // ongoing, process them now. - if (was_traversal && !ongoing_navigation.has()) { - while (!m_pending_navigations.is_empty()) { - auto navigation_params = m_pending_navigations.take_first(); - begin_navigation(navigation_params); - } - } + // ongoing, process them now. A freshly-created child navigable can also have pending navigations while + // its initial session history entry is being installed, so only drain once both gates are open. + if (was_traversal && !ongoing_navigation.has() && m_has_session_history_entry_and_ready_for_navigation) + process_pending_navigations(); } void Navigable::queue_pending_navigation(NavigateParams params, PendingNavigationBehavior behavior) @@ -753,6 +818,14 @@ void Navigable::queue_pending_navigation(NavigateParams params, PendingNavigatio m_pending_navigations.append(move(params)); } +void Navigable::process_pending_navigations() +{ + while (!m_pending_navigations.is_empty()) { + auto navigation_params = m_pending_navigations.take_first(); + begin_navigation(navigation_params); + } +} + // https://html.spec.whatwg.org/multipage/document-sequences.html#the-rules-for-choosing-a-navigable Navigable::ChosenNavigable Navigable::choose_a_navigable(StringView name, TokenizedFeature::NoOpener no_opener, ActivateTab activate_tab, Optional window_features) { @@ -2923,14 +2996,18 @@ void finalize_a_cross_document_navigation(GC::Ref navigable, HistoryH } else { target_entries_pointer = get_session_history_entries_if_present(*traversable, navigable); // https://html.spec.whatwg.org/multipage/browsing-the-web.html#getting-session-history-entries - // AD-HOC: The spec asserts that a nested history list is found. A queued child-frame commit can run - // after the iframe has been removed, when there is no remaining list to update. Chromium, - // WebKit, and Gecko bind child-frame commits to the live frame, so a removed frame's late - // commit has no observable session history effect. + // AD-HOC: The spec asserts that targetEntries is not null. A queued child-frame commit can run after the + // iframe was removed and its nested history list was pruned from the parent document state. Chromium, + // WebKit, and Gecko bind child-frame commits to the live frame, so detached frames have no observable + // session history effect. Conversely, if this is still the container's live content navigable, + // preserve the requested navigation by recreating the missing nested history. if (!target_entries_pointer) { - navigable->clear_navigation_load_event_guard(); - on_complete->function()(HistoryStepResult::Applied); - return; + target_entries_pointer = recreate_missing_nested_history_for_live_child_navigable(*traversable, *navigable); + if (!target_entries_pointer) { + navigable->clear_navigation_load_event_guard(); + on_complete->function()(HistoryStepResult::Applied); + return; + } } } auto& target_entries = *target_entries_pointer; @@ -3683,10 +3760,7 @@ void Navigable::stop_loading() void Navigable::set_has_session_history_entry_and_ready_for_navigation() { m_has_session_history_entry_and_ready_for_navigation = true; - while (!m_pending_navigations.is_empty()) { - auto navigation_params = m_pending_navigations.take_first(); - begin_navigation(navigation_params); - } + process_pending_navigations(); } Painting::CompositorSurfaceId Navigable::compositor_surface_id() const diff --git a/Libraries/LibWeb/HTML/Navigable.h b/Libraries/LibWeb/HTML/Navigable.h index 9eb6241872..a1ee42e9bc 100644 --- a/Libraries/LibWeb/HTML/Navigable.h +++ b/Libraries/LibWeb/HTML/Navigable.h @@ -114,6 +114,7 @@ public: GC::Ptr active_window(); RefPtr get_the_target_history_entry(int target_step) const; + RefPtr get_the_target_history_entry_if_present(int target_step) const; void save_persisted_state_to_active_session_history_entry(); void restore_persisted_state_from_session_history_entry(SessionHistoryEntry const&); @@ -311,6 +312,7 @@ private: void begin_navigation(NavigateParams); void queue_pending_navigation(NavigateParams, PendingNavigationBehavior); + void process_pending_navigations(); void navigate_to_a_fragment(URL::URL const&, HistoryHandlingBehavior, UserNavigationInvolvement, GC::Ptr source_element, Optional navigation_api_state, String navigation_id); void navigate_to_a_javascript_url(URL::URL const&, HistoryHandlingBehavior, GC::Ref, URL::Origin const& initiator_origin, UserNavigationInvolvement, ContentSecurityPolicy::Directives::Directive::NavigationType csp_navigation_type, InitialInsertion, String navigation_id); @@ -420,6 +422,8 @@ private: WEB_API HashTable>& all_navigables(); +Vector>* append_nested_history_for_child_navigable( + Navigable& parent_navigable, Navigable& child_navigable, SessionHistoryEntry& history_entry); bool navigation_must_be_a_replace(URL::URL const& url, DOM::Document const& document); void finalize_a_cross_document_navigation(GC::Ref, HistoryHandlingBehavior, UserNavigationInvolvement, NonnullRefPtr, GC::Ptr pending_document, Optional expected_ongoing_navigation_id, GC::Ref on_complete); void perform_url_and_history_update_steps(DOM::Document& document, URL::URL new_url, Optional = {}, HistoryHandlingBehavior history_handling = HistoryHandlingBehavior::Replace); diff --git a/Libraries/LibWeb/HTML/NavigableContainer.cpp b/Libraries/LibWeb/HTML/NavigableContainer.cpp index b0d0e15023..5794ae325f 100644 --- a/Libraries/LibWeb/HTML/NavigableContainer.cpp +++ b/Libraries/LibWeb/HTML/NavigableContainer.cpp @@ -119,28 +119,8 @@ void NavigableContainer::create_new_child_navigable() return; } - // 1. Let parentDocState be parentNavigable's active session history entry's document state. - auto parent_doc_state = parent_navigable->active_session_history_entry()->document_state(); - - // 2. Let parentNavigableEntries be the result of getting session history entries for parentNavigable. - auto parent_navigable_entries = parent_navigable->get_session_history_entries(); - - // 3. Let targetStepSHE be the first session history entry in parentNavigableEntries whose document state equals parentDocState. - auto target_step_she = *parent_navigable_entries.find_if([parent_doc_state](auto& entry) { - return entry->document_state() == parent_doc_state; - }); - - // 4. Set historyEntry's step to targetStepSHE's step. - history_entry->set_step(target_step_she->step()); - - // 5. Let nestedHistory be a new nested history whose id is navigable's id and entries list is « historyEntry ». - DocumentState::NestedHistory nested_history { - .id = navigable->id(), - .entries { *history_entry }, - }; - - // 6. Append nestedHistory to parentDocState's nested histories. - parent_doc_state->nested_histories().append(move(nested_history)); + // 1-6. Append nestedHistory to parentDocState's nested histories. + VERIFY(append_nested_history_for_child_navigable(*parent_navigable, *navigable, *history_entry)); // 7. Update for navigable creation/destruction given traversable traversable->update_for_navigable_creation_or_destruction(GC::create_function(traversable->heap(), [signal](HistoryStepResult) { @@ -221,15 +201,6 @@ Optional NavigableContainer::shared_attribute_processing_steps_for_ifr if (!m_content_navigable) return {}; - // AD-HOC: If the content navigable already has a navigation in progress or pending, - // skip the initial attribute processing. Without this, the about:blank URL update - // from perform_url_and_history_update_steps creates a state machine that clobbers the - // navigable's ongoing_navigation, causing the real navigation to be dropped when its - // populate completion callback checks ongoing_navigation != navigation_id. - if (initial_insertion == InitialInsertion::Yes && (m_content_navigable->has_pending_navigations() || !m_content_navigable->ongoing_navigation().has())) { - return {}; - } - // 1. Let url be the URL record about:blank. auto url = URL::about_blank(); @@ -252,6 +223,17 @@ Optional NavigableContainer::shared_attribute_processing_steps_for_ifr return {}; } + // AD-HOC: If the content navigable already has a navigation in progress or pending, skip the initial + // about:blank URL update. Without this, the URL update creates a state machine that clobbers the + // navigable's ongoing_navigation, causing the real navigation to be dropped when its populate completion + // callback checks ongoing_navigation != navigation_id. Non-blank src navigations must still be processed + // here, and will be queued by Navigable::navigate() until the child navigable is ready for navigation. + if (url_matches_about_blank(url) && initial_insertion == InitialInsertion::Yes + && (m_content_navigable->has_pending_navigations() + || !m_content_navigable->ongoing_navigation().has())) { + return {}; + } + // 4. If url matches about:blank and initialInsertion is true, then perform the URL and history update steps given element's content navigable's active document and url. if (url_matches_about_blank(url) && initial_insertion == InitialInsertion::Yes) { auto& document = *m_content_navigable->active_document(); diff --git a/Libraries/LibWeb/HTML/TraversableNavigable.cpp b/Libraries/LibWeb/HTML/TraversableNavigable.cpp index 40f1a0df7d..f08a358d54 100644 --- a/Libraries/LibWeb/HTML/TraversableNavigable.cpp +++ b/Libraries/LibWeb/HTML/TraversableNavigable.cpp @@ -601,7 +601,7 @@ Vector> TraversableNavigable::get_all_navigables_whose_curre auto navigable = navigables_to_check.take_first(); // 1. Let targetEntry be the result of getting the target history entry given navigable and targetStep. - auto target_entry = navigable->get_the_target_history_entry(target_step); + auto target_entry = navigable->get_the_target_history_entry_if_present(target_step); if (!target_entry) continue; @@ -641,7 +641,7 @@ Vector> TraversableNavigable::get_all_navigables_that_only_n auto navigable = navigables_to_check.take_first(); // 1. Let targetEntry be the result of getting the target history entry given navigable and targetStep. - auto target_entry = navigable->get_the_target_history_entry(target_step); + auto target_entry = navigable->get_the_target_history_entry_if_present(target_step); if (!target_entry) continue; @@ -680,7 +680,7 @@ Vector> TraversableNavigable::get_all_navigables_that_might_ auto navigable = navigables_to_check.take_first(); // 1. Let targetEntry be the result of getting the target history entry given navigable and targetStep. - auto target_entry = navigable->get_the_target_history_entry(target_step); + auto target_entry = navigable->get_the_target_history_entry_if_present(target_step); if (!target_entry) continue; @@ -980,7 +980,7 @@ void ApplyHistoryStepState::start() continue; // 1. Let targetEntry be the result of getting the target history entry given navigable and targetStep. - auto target_entry = navigable->get_the_target_history_entry(m_target_step); + auto target_entry = navigable->get_the_target_history_entry_if_present(m_target_step); if (!target_entry) continue; diff --git a/Tests/LibWeb/Text/expected/navigation/iframe-load-after-remove-and-recreate-same-src.txt b/Tests/LibWeb/Text/expected/navigation/iframe-load-after-remove-and-recreate-same-src.txt new file mode 100644 index 0000000000..9f93138a2b --- /dev/null +++ b/Tests/LibWeb/Text/expected/navigation/iframe-load-after-remove-and-recreate-same-src.txt @@ -0,0 +1,2 @@ +first load +second load diff --git a/Tests/LibWeb/Text/expected/navigation/iframe-load-after-remove-and-recreate-with-pending-history.txt b/Tests/LibWeb/Text/expected/navigation/iframe-load-after-remove-and-recreate-with-pending-history.txt new file mode 100644 index 0000000000..9f93138a2b --- /dev/null +++ b/Tests/LibWeb/Text/expected/navigation/iframe-load-after-remove-and-recreate-with-pending-history.txt @@ -0,0 +1,2 @@ +first load +second load diff --git a/Tests/LibWeb/Text/expected/navigation/iframe-pushstate-remove-recreate.txt b/Tests/LibWeb/Text/expected/navigation/iframe-pushstate-remove-recreate.txt new file mode 100644 index 0000000000..7ef22e9a43 --- /dev/null +++ b/Tests/LibWeb/Text/expected/navigation/iframe-pushstate-remove-recreate.txt @@ -0,0 +1 @@ +PASS diff --git a/Tests/LibWeb/Text/input/navigation/iframe-load-after-remove-and-recreate-same-src.html b/Tests/LibWeb/Text/input/navigation/iframe-load-after-remove-and-recreate-same-src.html new file mode 100644 index 0000000000..c08181353d --- /dev/null +++ b/Tests/LibWeb/Text/input/navigation/iframe-load-after-remove-and-recreate-same-src.html @@ -0,0 +1,30 @@ + + + diff --git a/Tests/LibWeb/Text/input/navigation/iframe-load-after-remove-and-recreate-with-pending-history.html b/Tests/LibWeb/Text/input/navigation/iframe-load-after-remove-and-recreate-with-pending-history.html new file mode 100644 index 0000000000..0a57ddd551 --- /dev/null +++ b/Tests/LibWeb/Text/input/navigation/iframe-load-after-remove-and-recreate-with-pending-history.html @@ -0,0 +1,37 @@ + + + diff --git a/Tests/LibWeb/Text/input/navigation/iframe-pushstate-remove-recreate.html b/Tests/LibWeb/Text/input/navigation/iframe-pushstate-remove-recreate.html new file mode 100644 index 0000000000..6a7ce92ecd --- /dev/null +++ b/Tests/LibWeb/Text/input/navigation/iframe-pushstate-remove-recreate.html @@ -0,0 +1,29 @@ + + +