QWebPage::acceptNavigationRequest type is not QWebPage::NavigationTypeReload when QWebPage::reload() is called

• Component: Page Loading
• Status: RESOLVED INVALID

[Benjamin Meyer]

After calling QWebPage::reload() QWebPage::acceptNavigationRequest() is called, but the type is not WebPage::NavigationTypeReload, but is QWebPage::Other

In FrameLoader.cpp FrameLoader::reload should it always call setTriggeringAction()?  Currently it is only calling it if request.httpMethod() == "POST".  This causes dispatchDecidePolicyForNavigationAction to to only now that it is a reload when it is a post.

Below it the initial patch that solves this.  If this is correct I can turn this into a proper patch.

--- a/src/3rdparty/webkit/WebCore/loader/FrameLoader.cpp
+++ b/src/3rdparty/webkit/WebCore/loader/FrameLoader.cpp
@@ -2456,9 +2456,10 @@ void FrameLoader::reload(bool endToEndReload)
     // If we're about to re-post, set up action so the application can warn the user.
     if (request.httpMethod() == "POST")
         loader->setTriggeringAction(NavigationAction(request.url(), NavigationTypeFormResubmitted));
+    else
+        loader->setTriggeringAction(NavigationAction(request.url(),
+                    endToEndReload ? FrameLoadTypeReloadFromOrigin : FrameLoadTypeReload, 0));

     loader->setOverrideEncoding(m_documentLoader->overrideEncoding());
     loadWithDocumentLoader(loader.get(), endToEndReload ? FrameLoadTypeReloadFromOrigin : FrameLoadTypeReload, 0);
 }

[Benjamin Meyer]

Created attachment 28183 [details]
Fix with test

[Adam Treat]

This looks correct in principle to me although I wonder if it'll cause a behavior change in Safari.  Would feel better if Darin or someone more familiar with loader could look at it.

[Adam Treat]

After talking with ap and anttik on IRC we agreed the patch should be amended to use NavigationType instead of FrameLoadType enum in the NavigationAction ctor.  Also, you should amend the in-code comment to explain what is happening and why.

Finally, can you give a real world case where an application would need to be able to distinguish between Reload and Other for purposes of deciding the navigation policy as this might have unintended consequences for other API's and their clients. (e.g. why the Qt port needs to distinguish this)

[Kent Hansen]

Any plans to proceed with this patch?

[Benjamin Meyer]

I want to say I wrote this patch when trying to implement the reload and shift-reload feature for arora.

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.