From cf6768d81ed7b9444e28ede6e26e125d5919b143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCri=20Valdmann?= Date: Mon, 16 Oct 2017 12:54:35 +0200 Subject: Stop preserving aborted navigation entries Consider the scenario 1. user enters url "http://localhost:8000/" A new navigation entry is created and committed in the NavigationController. 2. user enters url "http://localhost:8000/download.bin" A new navigation entry is created and a download is triggered, but the pending navigation entry in the NavigationController is neither committed nor discarded (since our WebContentsDelegate's ShouldPreserveAbortedURLs() returns true). 3. user enters url "http://localhost:8000/download.bin" At this point the NavigationController will have "http://localhost:8000/" as the committed navigation entry and "http://localhost:8000/download.bin" as the pending entry. NavigateToPendingEntry will see that the user is trying to navigate again to the same URL as the last pending entry and will therefore identify this new navigation as a reload. However Blink interprets 'reload' to mean reloading the last committed entry, i.e. "http://localhost:8000/", and so we end up trying to download "http://localhost:8000/" instead of "http://localhost:8000/download.bin" as the user might have expected. The patch removes the ShouldPreserveAbortedURLs override and relies on the default implementation which always returns false. As a result the pending navigation entry in step 2 above is discarded once the download has been triggered and the unexpected behavior in step 3 is no longer triggered. Removing the override resurrects QTBUG-48995 where, for example, calling QWebEnginePage::setUrl triggers first a urlChanged signal for the *old* URL. The patch adds url and title properties to WebContentsDelegateQt so that property change signals are triggered only if the properties have actually changed. A consequence of this fix is that the first urlChanged signal is delivered directly from the setUrl/load method and not asynchronously once the loading starts (this is also how Chrome's URL bar is updated). Task-number: QTBUG-63388 Change-Id: Icfa300b165e5e56f1fbc8978a00a237c263df183 Reviewed-by: Peter Varga --- src/core/web_contents_delegate_qt.cpp | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) (limited to 'src/core/web_contents_delegate_qt.cpp') diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp index f35620c86..96c7c4c41 100644 --- a/src/core/web_contents_delegate_qt.cpp +++ b/src/core/web_contents_delegate_qt.cpp @@ -131,10 +131,21 @@ content::WebContents *WebContentsDelegateQt::OpenURLFromTab(content::WebContents void WebContentsDelegateQt::NavigationStateChanged(content::WebContents* source, content::InvalidateTypes changed_flags) { - if (changed_flags & content::INVALIDATE_TYPE_URL) - m_viewClient->urlChanged(toQt(source->GetVisibleURL())); - if (changed_flags & content::INVALIDATE_TYPE_TITLE) - m_viewClient->titleChanged(toQt(source->GetTitle())); + if (changed_flags & content::INVALIDATE_TYPE_URL) { + QUrl newUrl = toQt(source->GetVisibleURL()); + if (m_url != newUrl) { + m_url = newUrl; + m_viewClient->urlChanged(m_url); + } + } + + if (changed_flags & content::INVALIDATE_TYPE_TITLE) { + QString newTitle = toQt(source->GetTitle()); + if (m_title != newTitle) { + m_title = newTitle; + m_viewClient->titleChanged(m_title); + } + } // NavigationStateChanged gets called with INVALIDATE_TYPE_TAB by AudioStateProvider::Notify, // whenever an audio sound gets played or stopped, this is the only way to actually figure out @@ -146,20 +157,6 @@ void WebContentsDelegateQt::NavigationStateChanged(content::WebContents* source, } } -bool WebContentsDelegateQt::ShouldPreserveAbortedURLs(content::WebContents *source) -{ - Q_UNUSED(source) - - // Allow failed URLs to stick around in the URL bar, but only when the error-page is enabled. - WebEngineSettings *settings = m_viewClient->webEngineSettings(); - bool isErrorPageEnabled = settings->testAttribute(settings->Attribute::ErrorPageEnabled); - - if (isErrorPageEnabled) - return true; - - return false; -} - void WebContentsDelegateQt::AddNewContents(content::WebContents* source, content::WebContents* new_contents, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture, bool* was_blocked) { Q_UNUSED(source) -- cgit v1.2.3