Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

File extensions not auto added to some downloads #1985

Closed
bbondy opened this issue May 30, 2016 · 18 comments · Fixed by #11906
Closed

File extensions not auto added to some downloads #1985

bbondy opened this issue May 30, 2016 · 18 comments · Fixed by #11906

Comments

@bbondy
Copy link
Member

bbondy commented May 30, 2016

Test plan

#11906 (comment)


I have a user report from support that file extensions are not being added for downloaded files. The user says they have to guess the file extension so the file gets saved.

@bbondy bbondy added the needs-info Another team member needs information from the PR/issue opener. label May 30, 2016
@luixxiul
Copy link
Contributor

luixxiul commented Oct 1, 2016

Actually extensions are added automatically when downloading, but they are cleared if you change the file name of it.

STR:

  1. On Windows, download a file without changing the default file name
  2. Make sure the file extension is added automatically and retained
  3. Download a file, with changing the file name
  4. Make sure the default extension is cleared and you need to fill it by yourself, guessing which one it is.

I think this is a windows-only issue.

@luixxiul luixxiul added feature/download and removed needs-info Another team member needs information from the PR/issue opener. labels Jan 2, 2017
@luixxiul luixxiul added this to the Backlog milestone Jan 2, 2017
@ghost
Copy link

ghost commented Apr 21, 2017

+1

@luixxiul luixxiul added the bug label Jun 4, 2017
@srirambv
Copy link
Collaborator

srirambv commented Jun 6, 2017

@eljuno
Copy link
Contributor

eljuno commented Jun 20, 2017

+1 from community

@luixxiul
Copy link
Contributor

@jonathansampson
Copy link
Collaborator

jonathansampson commented Aug 28, 2017

This issue is indeed limited to Windows, and also affects the Save As… options.

I suspect the issue here is with the prompt receiving little more than a filename (source).

void WebContents::DownloadURL(const GURL& url, bool prompt_for_location) {
  auto browser_context = web_contents()->GetBrowserContext();
  auto download_manager = content::BrowserContext::GetDownloadManager(browser_context);
  auto params = content::DownloadUrlParameters::CreateForWebContentsMainFrame(web_contents(), url);
  if (prompt_for_location)
    params->set_prompt(prompt_for_location);
  download_manager->DownloadUrl(std::move(params));
}

In Brave, this results in a All Files dialog, whereas Chrome filters on the extension. Note the different values for Save as type in both of the screenshots below.

image

image

If you replace the filename in Brave, you have no way of telling what the proper extension ought to be. In Chrome, the extension will be enforced by the Save as type option, even if the new filename lacks an extension.

macOS pre-selects the filename up to, but not including, the extension. This reduces the likelihood that the user will unintentionally mess up the extension.

image

A showSaveDialog method is defined in Electron, and preserved in Muon, that we might prefer. This method accepts a filters argument, allowing the Save As dialog to focus exclusively on particular file types. Presumably, this is more than just a way to filter what is shown, but to enforce what can be saved.

@jonathansampson jonathansampson modified the milestones: 0.20.x (Developer Channel), Backlog Aug 30, 2017
@jonathansampson
Copy link
Collaborator

Adding this to 0.20.x under the assumption that it's fairly straight-forward to use an alternative approach that supports explicit filter lists. If this isn't as straight-forward, please feel free to bump to another milestone.

@bsclifton
Copy link
Member

bsclifton commented Sep 5, 2017

When investigating this, it might be good to check this out: electron/electron#10121 (fixed with https://github.com/electron/electron/pull/10469/files)

@alexwykoff alexwykoff added muon priority/P3 Major loss of function. needs-STR This bug needs steps to reproduce. and removed needs-triage labels Sep 12, 2017
@luixxiul luixxiul removed the needs-STR This bug needs steps to reproduce. label Sep 14, 2017
@ghost ghost added needs-investigation A bug not 100% confirmed/fixed that needs QA to better audit. needs-STR This bug needs steps to reproduce. labels Sep 19, 2017
@bbondy bbondy modified the milestones: Backlog, Triage Backlog, Prioritized Backlog Oct 27, 2017
@jonathansampson
Copy link
Collaborator

+1 from Twitter: https://twitter.com/meta_gear/status/945817220944363522:

@brave Why does the browser not save files with their proper extensions? When I save an image it's just a generic file, not a .jpg or .png or whatever

@darkdh
Copy link
Member

darkdh commented Dec 27, 2017

Will be fixed by #11906

@jonathansampson
Copy link
Collaborator

@darkdh I was just checking out that PR :) Any idea when that will be merged?

@darkdh
Copy link
Member

darkdh commented Dec 27, 2017

It requires brave/muon#385 to be merged first. Muon PR is currently under reviewed

@eljuno
Copy link
Contributor

eljuno commented Jan 16, 2018

@darkdh darkdh modified the milestones: Backlog (Prioritized), 0.21.x (Developer Channel) Jan 27, 2018
@bsclifton
Copy link
Member

+1 from @bravefan via #12878

@srirambv
Copy link
Collaborator

Blocked on #13230 #13228

@bsclifton bsclifton modified the milestones: 0.22.x (Developer Channel), 0.21.x w/ Chromium 65 (Beta Channel) Mar 1, 2018
@bsclifton
Copy link
Member

Moving to 0.22.x; we're going to have 0.21.x-C65 only contain the Chromium upgrade 😄 👍

@bsclifton bsclifton modified the milestones: 0.21.x w/ Chromium 65 (Beta Channel), 0.22.x (Developer Channel) Mar 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.