From 007425cf163b61dcd41845f377fc74e1c89b6dd3 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 18 Feb 2016 16:17:35 +0100 Subject: [PATCH 1/3] downloads: fix filename for data: links Issue #1214 Now uses a sensible filename for data: links instead of the whole base64 content. For PDF.js, it even uses the correct pdf filename. TODO: Produces "QPainter::end: Painter ended with 2 saved states" while running the tests here (Arch Linux): CPython: 3.5.1 Qt: 5.5.1, runtime: 5.5.1 PyQt: 5.5.1 --- qutebrowser/browser/downloads.py | 20 ++++++++++++++++++- .../integration/data/downloads/issue1214.html | 10 ++++++++++ tests/integration/features/downloads.feature | 10 ++++++++++ tests/integration/features/misc.feature | 12 +++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 tests/integration/data/downloads/issue1214.html diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py index 8c977fb34..667e8e7a2 100644 --- a/qutebrowser/browser/downloads.py +++ b/qutebrowser/browser/downloads.py @@ -775,7 +775,25 @@ class DownloadManager(QAbstractListModel): request.setAttribute(QNetworkRequest.CacheLoadControlAttribute, QNetworkRequest.AlwaysNetwork) - suggested_fn = urlutils.filename_from_url(request.url()) + if request.url().scheme().lower() != 'data': + suggested_fn = urlutils.filename_from_url(request.url()) + else: + # We might be downloading a binary blob embedded on a page or even + # generated dynamically via javascript. We try to figure out a more + # sensible name than the base64 content of the data. + origin = request.originatingObject() + try: + origin_url = origin.url() + except AttributeError: + # Raised either if origin is None or some object that doesn't + # have its own url. We're probably fine with a default fallback + # then. + suggested_fn = 'binary blob' + else: + # Use the originating URL as a base for the filename (works + # e.g. for pdf.js). + suggested_fn = urlutils.filename_from_url(origin_url) + if suggested_fn is None: suggested_fn = 'qutebrowser-download' diff --git a/tests/integration/data/downloads/issue1214.html b/tests/integration/data/downloads/issue1214.html new file mode 100644 index 000000000..db56cdc61 --- /dev/null +++ b/tests/integration/data/downloads/issue1214.html @@ -0,0 +1,10 @@ + + + + + Wrong filename when using data: links + + + download + + diff --git a/tests/integration/features/downloads.feature b/tests/integration/features/downloads.feature index 5881767f6..d2ce1ce3c 100644 --- a/tests/integration/features/downloads.feature +++ b/tests/integration/features/downloads.feature @@ -34,6 +34,16 @@ Feature: Downloading things from a website. And I run :leave-mode Then no crash should happen + Scenario: Downloading a data: link (issue 1214) + When I set completion -> download-path-suggestion to filename + And I set storage -> prompt-download-directory to true + And I open data/downloads/issue1214.html + And I run :hint links download + And I run :follow-hint a + And I wait for "Asking question text='Save file to:'>, *" in the log + And I run :leave-mode + Then no crash should happen + Scenario: Retrying a failed download When I run :download http://localhost:(port)/does-not-exist And I wait for the error "Download error: * - server replied: NOT FOUND" diff --git a/tests/integration/features/misc.feature b/tests/integration/features/misc.feature index 7a0f9ae2f..bec5a65b9 100644 --- a/tests/integration/features/misc.feature +++ b/tests/integration/features/misc.feature @@ -290,6 +290,18 @@ Feature: Various utility commands. And I open data/misc/test.pdf Then "Download finished" should be logged + Scenario: Downloading a pdf via pdf.js button (issue 1214) + Given pdfjs is available + When I set content -> enable-pdfjs to true + And I set completion -> download-path-suggestion to filename + And I set storage -> prompt-download-directory to true + And I open data/misc/test.pdf + And I wait for "[qute://pdfjs/*] PDF * (PDF.js: *)" in the log + And I run :jseval document.getElementById("download").click() + And I wait for "Asking question text='Save file to:'>, *" in the log + And I run :leave-mode + Then no crash should happen + # :print # Disabled because it causes weird segfaults and QPainter warnings in Qt... From a84c8ac247513318dce19a4b3c6f033e520a50d7 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Mon, 22 Feb 2016 17:39:34 +0100 Subject: [PATCH 2/3] tests: add workaround for QPainter bug As suggested in the github discussion. --- tests/integration/features/misc.feature | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/integration/features/misc.feature b/tests/integration/features/misc.feature index bec5a65b9..416836b3b 100644 --- a/tests/integration/features/misc.feature +++ b/tests/integration/features/misc.feature @@ -292,7 +292,11 @@ Feature: Various utility commands. Scenario: Downloading a pdf via pdf.js button (issue 1214) Given pdfjs is available - When I set content -> enable-pdfjs to true + # WORKAROUND to prevent the "Painter ended with 2 saved states" warning + # Might be related to https://bugreports.qt.io/browse/QTBUG-13524 and + # a weird interaction with the previous test. + When I wait 0.5s + And I set content -> enable-pdfjs to true And I set completion -> download-path-suggestion to filename And I set storage -> prompt-download-directory to true And I open data/misc/test.pdf From a382b366bcd0e2ddff71f07ae701626f0ba48e00 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Mon, 22 Feb 2016 21:46:54 +0100 Subject: [PATCH 3/3] tests: increase wait time Otherwise the test might still fail on some systems --- tests/integration/features/misc.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/features/misc.feature b/tests/integration/features/misc.feature index 416836b3b..8fa35c558 100644 --- a/tests/integration/features/misc.feature +++ b/tests/integration/features/misc.feature @@ -295,7 +295,7 @@ Feature: Various utility commands. # WORKAROUND to prevent the "Painter ended with 2 saved states" warning # Might be related to https://bugreports.qt.io/browse/QTBUG-13524 and # a weird interaction with the previous test. - When I wait 0.5s + When I wait 1s And I set content -> enable-pdfjs to true And I set completion -> download-path-suggestion to filename And I set storage -> prompt-download-directory to true