1651175 - 'Save As' dialog shows incorrect saved file type when using downloads.download API

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect, P5)

Description

[dw-dev]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

I am the developer of the Save Page WE extension. Until recently, Save Page WE saved the .html file using the HTML5 download attribute. Since Version 20.0, Save Page WE has had an option to save the .html file using the downloads.download API.

Normally, the Firefox 'Save As' dialog shows the 'Saved file type' based on the filename extension. When saving the .html file using the HTML5 download attribute, the 'Saved file type' is shown as "Firefox HTML Document (.html)". However, when saving the .html file using the downloads.download API, the 'Saved file type' is shown as "All Files (.*)".

Note, when using Save Page WE in Chrome, the 'Saved file type' is always shown as "HTML Document (*.html)", whether saving the .html file using the HTML5 download attribute or using the downloads.download API,

Actual results:

When saving a .html file using the downloads.download API, the 'Saved file type' is shown as "All Files (.)".

Expected results:

When saving a .html file using the downloads.download API, the 'Saved file type' should be shown as "Firefox HTML Document (*.html)".

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Tomislav Jovanovic :zombie]

Is this a recent regression, or has this always worked like that?

Comment 3

[Rob Wu [:robwu]]

Until recently,

Could you provide the regression range please? With mozregression for example.

Comment 4

[dw-dev]

I have just tested Save Page WE with Firefox 58 and the same problem exists. So I'm pretty sure that it has always worked like this.

Comment 5

[Rob Wu [:robwu]]

Thanks for checking.

The downloads API implementation sets the filter here:
https://searchfox.org/mozilla-central/rev/8d55e18875b89cdf2a22a7cba60dc40999c18356/toolkit/components/extensions/parent/ext-downloads.js#800

I'm willing to accept a patch that adds more types depending on the file extension. More standard filters are available here: https://searchfox.org/mozilla-central/rev/8d55e18875b89cdf2a22a7cba60dc40999c18356/widget/nsIFilePicker.idl#38

Comment 6

[dw-dev]

I've done some more reading about nsIFilePicker filters, but they don't seem to work the way I was expecting. I had assumed that if you passed in a list of filters, then the file picker would look at the filename extension and automatically apply the relevant filter. However, it appears that the first filter in the list is always applied by default, and that the user would have to manually apply any other filter. Since the downloads.download() API would be expected to save files with a large variety of filename extensions, the only sensible choice for the default filter is "All Files".

I have searched through the Firefox code base on DXR and I can only find one example of automatically applying the relevant filter. This is done by function appendFiltersForContentType() in contentAreaAreaUtils.js:
https://dxr.mozilla.org/mozilla-release/source/toolkit/content/contentAreaUtils.js#1020

This adds a default filter based on the content type, filename extension and save mode - and also adds the "All Files" filter. We could use a simplified version of this function - we would't need the WebPageComplete filters and only need to handle SAVEMODE_FILEONLY - so we just need lines 1063-1075 and 1097-1098.

Does this make sense or am I missing something?

Comment 7

[dw-dev]

Rob, I meant to add a 'needinfo' request on Comment 6 to get your view on whether the proposl is sensible.

Comment 8

[Rob Wu [:robwu]]

I was thinking of comparing the file extension to the list of known extensions* and then adding the relevant filter, but what you're suggesting looks reasonable too.

* The list of file extensions for each label is listed at toolkit/content/filepicker.properties.

Comment 9

[dw-dev]

Attachment: Bug 1651175 Make downloads.download() show specific saved file type; r?robwu

Comment 10

[dw-dev]

Rob, I have gone for the approach you suggested.

I did notice a few minor differences between the lists of extension types in widget/nsIFilePicker.idl and toolkit/content/filepicker.properties. Specifically, "*.shtml", "*.xhtml" and "*.text" are missing from the former.

I have run all of the downloads API tests (4 mochitest, 6 xpcshell-test) to make sure they still pass with no errors.

I have manually tested downloads.download() saving files with extensions from each of the seven filter types (filterHTML, filterText, filterImages, filterXUL, filterXML, filterAudio and filterVideo).

I have just noticed that in function appendFilterForFileExtension() the XUL filter appears before the XML filter. I can change this for consistent ordering everwhere if you want?

Comment 11

[Rob Wu [:robwu]]

I have just noticed that in function appendFilterForFileExtension() the XUL filter appears before the XML filter. I can change this for consistent ordering everwhere if you want?

I replied in the patch and suggested to just remove .xul.

Comment 12

[dw-dev]

Rob, I have submitted a new revision for review, which addresses your comments and adds a mochitest test file with tests for each filter.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f5df865967c9
Make downloads.download() show specific saved file type; r=robwu

Comment 14

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/f5df865967c9