WebKitDownload provides a "suggested filename". This works often enough but sometimes it suggests something like "jdk-6u13-windows-i586-p.exe&File=jdk-6u13-windows-i586-p.exe" which is obviously not ideal. Another phenomenon seems to be URI encoded spaces, ie. %20, which is also not useful. I think we should try to catch these cases.
Created attachment 29176 [details] Better handling of the URI that is to be downloaded to figure out a name suggestion. --- WebKit/gtk/ChangeLog | 13 +++++++++++++ WebKit/gtk/webkit/webkitdownload.cpp | 6 +++++- 2 files changed, 18 insertions(+), 1 deletions(-)
Comment on attachment 29176 [details] Better handling of the URI that is to be downloaded to figure out a KURL has all we need =). We can even use the KURL the request itself holds in the future, when we have a proper WebKitNetworkRequest implementation committed =).
Created attachment 29254 [details] Refactor the emission of the download-requested signal so that we have less code duplication. WebKit/gtk/ChangeLog | 17 +++++++++++++++++ WebKit/gtk/WebCoreSupport/ContextMenuClientGtk.cpp | 16 +++------------- WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp | 14 ++------------ WebKit/gtk/webkit/webkitprivate.h | 4 ++++ WebKit/gtk/webkit/webkitwebview.cpp | 18 ++++++++++++++++++ 5 files changed, 44 insertions(+), 25 deletions(-)
Created attachment 29255 [details] When a download is requested by an ongoing request, use the already provided response to set the suggested filename for the WebKitDownload object, if available. --- WebKit/gtk/ChangeLog | 20 ++++++++++++++++++ WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp | 9 ++++++- WebKit/gtk/webkit/webkitdownload.cpp | 22 ++++++++++++++----- WebKit/gtk/webkit/webkitprivate.h | 5 +++- WebKit/gtk/webkit/webkitwebview.cpp | 7 +++++- 5 files changed, 53 insertions(+), 10 deletions(-)
Created attachment 29257 [details] When a download is requested by an ongoing request, use the already provided response to set the suggested filename for the WebKitDownload object, if available. --- WebKit/gtk/ChangeLog | 20 +++++++++++++++++ WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp | 9 ++++++- WebKit/gtk/webkit/webkitdownload.cpp | 23 +++++++++++++------ WebKit/gtk/webkit/webkitprivate.h | 5 +++- WebKit/gtk/webkit/webkitwebview.cpp | 7 +++++- 5 files changed, 53 insertions(+), 11 deletions(-)
Comment on attachment 29176 [details] Better handling of the URI that is to be downloaded to figure out a Looks good to me. Maybe you can briefly say in the changelog how is it better than before (no query, reference, escaping, etc).
Comment on attachment 29254 [details] Refactor the emission of the download-requested signal so that we have less code duplication. > void ContextMenuClient::downloadURL(const KURL& url) > { > - WebKitNetworkRequest* network_request = webkit_network_request_new(url.string().utf8().data()); > - WebKitDownload* download = webkit_download_new(network_request); > - g_object_unref(network_request); > + WebKitNetworkRequest* networkRequest = webkit_network_request_new(url.string().utf8().data()); > > - gboolean handled; > - g_signal_emit_by_name(m_webView, "download-requested", download, &handled); > - > - if (!handled) { > - webkit_download_cancel(download); > - g_object_unref(download); > - return; > - } > - > - webkit_download_start(download); > + webkit_web_view_request_download(m_webView, networkRequest); > + g_object_unref(networkRequest); > } > +void webkit_web_view_request_download(WebKitWebView* webView, WebKitNetworkRequest* request) > +{ > + g_return_if_fail(WEBKIT_IS_WEB_VIEW(webView)); > + > + WebKitDownload* download = webkit_download_new(request); > + g_object_unref(request); > + Mmm, aren't you unrefing the network request twice now? Once in the request_download function, and once in the functions called that.
Comment on attachment 29176 [details] Better handling of the URI that is to be downloaded to figure out a Landed as r42672 with a more descriptive changelog message.
Created attachment 29617 [details] refactor download-requested signal emission to avoid code duplication Xan's right, I'm not sure why I didn't get any crashers while testing, though I don't remember how/if I tested doing the actual downloads.
Comment on attachment 29617 [details] refactor download-requested signal emission to avoid code duplication Go for it.
Comment on attachment 29617 [details] refactor download-requested signal emission to avoid code duplication Landed as r42867.
Comment on attachment 29257 [details] When a download is requested by an ongoing request, use the already > + if (!response.isNull() && !response.suggestedFilename().isEmpty()) { > + g_debug("suggested filename exists!"); > + webkit_download_set_suggested_filename(download, response.suggestedFilename().utf8().data()); > + } > + I guess you want to lose the g_debug. Looks good otherwise.
Last patch landed as r43319.