From 625e7d59d2e8efdd50d704e76057a48eb8ee5b93 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gorszkowski Date: Tue, 9 Jul 2024 11:58:18 +0200 Subject: [PATCH 1/2] Do not stop navigation when fragment navigation starts --- Source/WebCore/loader/FrameLoader.cpp | 43 ++++++++++++++++----------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp index 8b594f8cca85f..d081efa34249a 100644 --- a/Source/WebCore/loader/FrameLoader.cpp +++ b/Source/WebCore/loader/FrameLoader.cpp @@ -1482,23 +1482,6 @@ void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& ref if (m_inStopAllLoaders || m_inClearProvisionalLoadForPolicyCheck) return; - static bool keepNavigationOnFragmentLoad = false; - static bool keepNavigationOnFragmentLoadInitialized = false; - - if (!keepNavigationOnFragmentLoadInitialized) { - keepNavigationOnFragmentLoad = !!getenv("WPE_KEEP_NAVIGATION_ON_FRAGMENT_LOAD"); - keepNavigationOnFragmentLoadInitialized = true; - } - - // If we have a policy or provisional request for a different document, a fragment scroll should be cancelled. - if (keepNavigationOnFragmentLoad && (m_policyDocumentLoader && !equalIgnoringFragmentIdentifier(m_policyDocumentLoader->request().url(), frameLoadRequest.resourceRequest().url()) || - m_provisionalDocumentLoader && !equalIgnoringFragmentIdentifier(m_provisionalDocumentLoader->request().url(), frameLoadRequest.resourceRequest().url()))) { - const auto fragmentNavigationURL = frameLoadRequest.resourceRequest().url(); - const auto navigationURL = m_policyDocumentLoader ? m_policyDocumentLoader->request().url(): m_provisionalDocumentLoader->request().url(); - FRAMELOADER_RELEASE_LOG(ResourceLoading, "loadURL: fragment navigation: %s is cancelled because of ongoing navigation change to url: %s", fragmentNavigationURL.string().utf8().data(), navigationURL.string().utf8().data()); - return; - } - Ref frame = m_frame.get(); // Anchor target is ignored when the download attribute is set since it will download the hyperlink rather than follow it. @@ -1577,15 +1560,39 @@ void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& ref if (!dispatchNavigateEvent(newURL, newLoadType, action, frameLoadRequest.navigationHistoryBehavior(), true)) return; + static bool keepNavigationOnFragmentLoad = false; + static bool keepNavigationOnFragmentLoadInitialized = false; + + if (!keepNavigationOnFragmentLoadInitialized) { + keepNavigationOnFragmentLoad = !!getenv("WPE_KEEP_NAVIGATION_ON_FRAGMENT_LOAD"); + keepNavigationOnFragmentLoadInitialized = true; + } oldDocumentLoader->setTriggeringAction(WTFMove(action)); oldDocumentLoader->setLastCheckedRequest(ResourceRequest()); - policyChecker().stopCheck(); + auto loadType = policyChecker().loadType(); + bool resetLoadTypeAfterFragmentNavigation = false; + if (keepNavigationOnFragmentLoad && (m_policyDocumentLoader && !equalIgnoringFragmentIdentifier(m_policyDocumentLoader->request().url(), frameLoadRequest.resourceRequest().url()) || + m_provisionalDocumentLoader && !equalIgnoringFragmentIdentifier(m_provisionalDocumentLoader->request().url(), frameLoadRequest.resourceRequest().url()))) { + resetLoadTypeAfterFragmentNavigation = true; + + const auto fragmentNavigationURL = frameLoadRequest.resourceRequest().url(); + const auto navigationURL = m_policyDocumentLoader ? m_policyDocumentLoader->request().url(): m_provisionalDocumentLoader->request().url(); + FRAMELOADER_RELEASE_LOG(ResourceLoading, "loadURL: navigation to: %s will be continued after fragment navigation to url: %s", + navigationURL.string().utf8().data(), fragmentNavigationURL.string().utf8().data()); + } else { + policyChecker().stopCheck(); + } + policyChecker().setLoadType(newLoadType); RELEASE_ASSERT(!isBackForwardLoadType(newLoadType) || frame->history().provisionalItem()); policyChecker().checkNavigationPolicy(WTFMove(request), ResourceResponse { } /* redirectResponse */, oldDocumentLoader.get(), WTFMove(formState), [this, frame, requesterOrigin = Ref { frameLoadRequest.requesterSecurityOrigin() }, historyHandling = frameLoadRequest.navigationHistoryBehavior()] (const ResourceRequest& request, WeakPtr&&, NavigationPolicyDecision navigationPolicyDecision) { continueFragmentScrollAfterNavigationPolicy(request, requesterOrigin.ptr(), navigationPolicyDecision == NavigationPolicyDecision::ContinueLoad, historyHandling); }, PolicyDecisionMode::Synchronous); + + if (resetLoadTypeAfterFragmentNavigation) + policyChecker().setLoadType(loadType); + return; } From 5b7580e3257340dd77ff5a16e9dd51b40fa08a61 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gorszkowski Date: Tue, 30 Jul 2024 07:39:46 +0200 Subject: [PATCH 2/2] Use proper documentLoader for fragmentNavigation with ongoing async navigation --- Source/WebCore/loader/PolicyChecker.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Source/WebCore/loader/PolicyChecker.cpp b/Source/WebCore/loader/PolicyChecker.cpp index 15ce6dd0ee3c4..df6543780f71a 100644 --- a/Source/WebCore/loader/PolicyChecker.cpp +++ b/Source/WebCore/loader/PolicyChecker.cpp @@ -285,6 +285,10 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, const Resou } auto documentLoader = frameLoader->loaderForWebsitePolicies(); + // PolicyDecisionMode::Synchronous means that it is a FragmentNavigation and in that case we should use documentLoader, + // because there can be ongoing (in policy or provisional state) navigation. + if (policyDecisionMode == PolicyDecisionMode::Synchronous) + documentLoader = frameLoader->documentLoader(); auto clientRedirectSourceForHistory = documentLoader ? documentLoader->clientRedirectSourceForHistory() : String(); auto navigationID = documentLoader ? documentLoader->navigationID() : 0; bool hasOpener = !!frame->opener();