From d80d9eb26c6263a68a7bbbaf344afba3c5f282e9 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <me@the-compiler.org>
Date: Mon, 10 Sep 2018 13:15:39 +0200
Subject: [PATCH] Allow downloading from PDF.js

When we click the download button in PDF.js, it downloads a blob://qute:...
URL. We can detect that and force a download rather than opening it in PDF.js
again.

Note that what actually happens depends on the Qt version and backend:

QtWebKit (any Qt version):
Downloads always work properly (regardless of Qt version).

QtWebEngine, Qt 5.7.1:
Downloads work.

QtWebEngine, Qt 5.9 - 5.11:
Downloads won't work as we need to tell PDF.js to not use blob: URLs:
https://bugreports.qt.io/browse/QTBUG-70420 - in theory, PDF.js could fall back
to downloading the existing qute:// URL, but it has a whitelist of schemes
which does not include qute://... Since it's not in that whitelist, it just
ends up doing nothing at all.

QtWebEngine, Qt 5.12:
Downloads should hopefully work properly again, as we can register the qute://
scheme with Chromium, which allows us to use blob:// URLs.
---
 qutebrowser/browser/pdfjs.py                  |  9 ++++++---
 .../browser/webengine/webenginedownloads.py   |  2 +-
 qutebrowser/browser/webkit/webpage.py         |  2 +-
 tests/unit/browser/test_pdfjs.py              | 19 ++++++++++++-------
 4 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/qutebrowser/browser/pdfjs.py b/qutebrowser/browser/pdfjs.py
index da6dcbfe6..c6eef53d9 100644
--- a/qutebrowser/browser/pdfjs.py
+++ b/qutebrowser/browser/pdfjs.py
@@ -198,9 +198,12 @@ def is_available():
         return True
 
 
-def should_use_pdfjs(mimetype):
-    return (mimetype in ['application/pdf', 'application/x-pdf'] and
-            config.val.content.pdfjs)
+def should_use_pdfjs(mimetype, url):
+    # e.g. 'blob:qute%3A///b45250b3-787e-44d1-a8d8-c2c90f81f981'
+    is_download_url = (url.scheme() == 'blob' and
+                       QUrl(url.path()).scheme() == 'qute')
+    is_pdf = mimetype in ['application/pdf', 'application/x-pdf']
+    return is_pdf and not is_download_url and config.val.content.pdfjs
 
 
 def get_main_url(filename):
diff --git a/qutebrowser/browser/webengine/webenginedownloads.py b/qutebrowser/browser/webengine/webenginedownloads.py
index a9d44427d..3f47dd640 100644
--- a/qutebrowser/browser/webengine/webenginedownloads.py
+++ b/qutebrowser/browser/webengine/webenginedownloads.py
@@ -221,7 +221,7 @@ class DownloadManager(downloads.AbstractDownloadManager):
             download.set_target(self._mhtml_target)
             self._mhtml_target = None
             return
-        if pdfjs.should_use_pdfjs(qt_item.mimeType()):
+        if pdfjs.should_use_pdfjs(qt_item.mimeType(), qt_item.url()):
             download.set_target(downloads.PDFJSDownloadTarget())
             return
 
diff --git a/qutebrowser/browser/webkit/webpage.py b/qutebrowser/browser/webkit/webpage.py
index 7037f29fb..63017ec45 100644
--- a/qutebrowser/browser/webkit/webpage.py
+++ b/qutebrowser/browser/webkit/webpage.py
@@ -268,7 +268,7 @@ class BrowserPage(QWebPage):
             else:
                 reply.finished.connect(functools.partial(
                     self.display_content, reply, 'image/jpeg'))
-        elif pdfjs.should_use_pdfjs(mimetype):
+        elif pdfjs.should_use_pdfjs(mimetype, reply.url()):
             download_manager.fetch(reply,
                                    target=downloads.PDFJSDownloadTarget())
         else:
diff --git a/tests/unit/browser/test_pdfjs.py b/tests/unit/browser/test_pdfjs.py
index ff62545bd..91536e416 100644
--- a/tests/unit/browser/test_pdfjs.py
+++ b/tests/unit/browser/test_pdfjs.py
@@ -173,15 +173,20 @@ def test_is_available(available, mocker):
     assert pdfjs.is_available() == available
 
 
-@pytest.mark.parametrize('mimetype, enabled, expected', [
-    ('application/pdf', True, True),
-    ('application/x-pdf', True, True),
-    ('application/octet-stream', True, False),
-    ('application/pdf', False, False),
+@pytest.mark.parametrize('mimetype, url, enabled, expected', [
+    # PDF files
+    ('application/pdf', 'http://www.example.com', True, True),
+    ('application/x-pdf', 'http://www.example.com', True, True),
+    # Not a PDF
+    ('application/octet-stream', 'http://www.example.com', True, False),
+    # PDF.js disabled
+    ('application/pdf', 'http://www.example.com', False, False),
+    # Download button in PDF.js
+    ('application/pdf', 'blob:qute%3A///b45250b3', True, False),
 ])
-def test_should_use_pdfjs(mimetype, enabled, expected, config_stub):
+def test_should_use_pdfjs(mimetype, url, enabled, expected, config_stub):
     config_stub.val.content.pdfjs = enabled
-    assert pdfjs.should_use_pdfjs(mimetype) == expected
+    assert pdfjs.should_use_pdfjs(mimetype, QUrl(url)) == expected
 
 
 def test_get_main_url():