Summary: | WebKitDownload sometimes suggests peculiar filenames | ||
---|---|---|---|
Product: | WebKit | Reporter: | Christian Dywan <christian> |
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | gustavo, jobi |
Priority: | P2 | Keywords: | Gtk |
Version: | 528+ (Nightly build) | ||
Hardware: | Other | ||
OS: | Linux | ||
Attachments: |
Description
Christian Dywan
2009-03-24 14:44:03 PDT
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. |