Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use page title for filename with :download #2753

Merged
merged 8 commits into from Jul 9, 2017
Merged

Use page title for filename with :download #2753

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a24d7f6
Use page title for filename with :download
iordanisg Jun 26, 2017
8a5b48d
Add suggested_fn argument to get_request
iordanisg Jun 26, 2017
5e2be8a
Fix PEP-8 issue
iordanisg Jun 27, 2017
0a09758
Add file path to download
iordanisg Jun 27, 2017
57e4d49
Use page title only for whitelisted extensions
iordanisg Jul 6, 2017
3bfafb5
Refactor suggested_fn_from_title, add unit tests
iordanisg Jul 6, 2017
82d194c
Improve function docstring, add more tests
iordanisg Jul 6, 2017
c9fd182
Adjust suggested_fn_from_title, add tests
iordanisg Jul 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 17 additions & 2 deletions qutebrowser/browser/commands.py
View file
Open in desktop
Expand Up @@ -1443,8 +1443,23 @@ def download(self, url=None, dest_old=None, *, mhtml_=False, dest=None):
download_manager.get_mhtml(tab, target)
else:
qnam = tab.networkaccessmanager()
download_manager.get(self._current_url(), user_agent=user_agent,
qnam=qnam, target=target)

# Downloads of URLs with file extensions in the whitelist will use
# the page title as the filename.
ext_whitelist = [".html", ".htm", ".php", ""]
_, ext = os.path.splitext(self._current_url().path())
if ext.lower() in ext_whitelist and tab.title():
suggested_fn = utils.sanitize_filename(tab.title()) + ext
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When QtWebEngine auto-generates a title, this ends up as no title.html.html. I'd suggest checking for if not tab.title().endswith(ext): before adding it.

else:
suggested_fn = None

download_manager.get(
self._current_url(),
user_agent=user_agent,
qnam=qnam,
target=target,
suggested_fn=suggested_fn
)

@cmdutils.register(instance='command-dispatcher', scope='window')
def view_source(self):
Expand Down
7 changes: 5 additions & 2 deletions qutebrowser/browser/qtnetworkdownloads.py
View file
Open in desktop
Expand Up @@ -412,7 +412,8 @@ def get_mhtml(self, tab, target):
mhtml.start_download_checked, tab=tab))
message.global_bridge.ask(question, blocking=False)

def get_request(self, request, *, target=None, **kwargs):
def get_request(self, request, *, target=None,
suggested_fn=None, **kwargs):
"""Start a download with a QNetworkRequest.

Args:
Expand All @@ -428,7 +429,9 @@ def get_request(self, request, *, target=None, **kwargs):
request.setAttribute(QNetworkRequest.CacheLoadControlAttribute,
QNetworkRequest.AlwaysNetwork)

if request.url().scheme().lower() != 'data':
if suggested_fn is not None:
pass
elif 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
Expand Down
8 changes: 8 additions & 0 deletions tests/end2end/data/downloads/download with no title.html
View file
Open in desktop
@@ -0,0 +1,8 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
</head>
<body>
</body>
</html>
Binary file added tests/end2end/data/downloads/qutebrowser.png
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
23 changes: 22 additions & 1 deletion tests/end2end/features/downloads.feature
View file
Open in desktop
Expand Up @@ -22,6 +22,27 @@ Feature: Downloading things from a website.
And I wait until the download is finished
Then the downloaded file download.bin should exist

Scenario: Using :download with no URL
When I set storage -> prompt-download-directory to false
And I open data/downloads/downloads.html
And I run :download
And I wait until the download is finished
Then the downloaded file Simple downloads.html should exist

Scenario: Using :download with no URL on an image
When I set storage -> prompt-download-directory to false
And I open data/downloads/qutebrowser.png
And I run :download
And I wait until the download is finished
Then the downloaded file qutebrowser.png should exist

Scenario: Using :download with no URL and no page title
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like QtWebEngine autogenerates a filename while QtWebKit doesn't, which also explains that you don't see it locally. After adding some unittests (see my review comment), I'd suggest just removing this end2end test entirely (but keep the other two).

When I set storage -> prompt-download-directory to false
And I open data/downloads/download with no title.html
And I run :download
And I wait until the download is finished
Then the downloaded file download with no title.html should exist

Scenario: Using hints
When I set storage -> prompt-download-directory to false
And I open data/downloads/downloads.html
Expand Down Expand Up @@ -637,7 +658,7 @@ Feature: Downloading things from a website.
@qtwebengine_skip: We can't get the UA from the page there
Scenario: user-agent when using :download
When I open user-agent
And I run :download
And I run :download --dest user-agent
And I wait until the download is finished
Then the downloaded file user-agent should contain Safari/

Expand Down