Rename getUserDownloadsDirectory

RESOLVED FIXED in mozilla27

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Paolo, Assigned: raymondlee)

Tracking

(Blocks: 1 bug)

Trunk
mozilla27
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
The getUserDownloadsDirectory and getTemporaryDownloadsDirectory functions of
the Downloads API were named after the methods of nsIDownloadManager, but now
we must find new names that are more indicative of what the functions do.
(Reporter)

Updated

5 years ago
Blocks: 875731
I suggested getPreferredDownloadsDirectory, to replace getUser...
But I don't have any good ideas for the Temporary directory so far, nothing better than temporary, at least.
(Assignee)

Comment 2

5 years ago
Created attachment 807033 [details] [diff] [review]
Change getUserDownloadsDirectory to getPreferredDownloadsDirectory
Attachment #807033 - Flags: review?(paolo.mozmail)
(Reporter)

Comment 3

5 years ago
Comment on attachment 807033 [details] [diff] [review]
Change getUserDownloadsDirectory to getPreferredDownloadsDirectory

Review of attachment 807033 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/jsdownloads/test/unit/test_DownloadIntegration.js
@@ +192,1 @@
>      do_check_eq(downloadDir.path, userDownloadDir.path);

nit: "preferredDownloadDir"
Attachment #807033 - Flags: review?(paolo.mozmail) → review+
(Reporter)

Comment 4

5 years ago
(In reply to Marco Bonardo [:mak] from comment #1)
> I suggested getPreferredDownloadsDirectory, to replace getUser...
> But I don't have any good ideas for the Temporary directory so far, nothing
> better than temporary, at least.

We currently invoke functions called "deleteTemporaryFileOnExit" and "deleteTemporaryPrivateFileWhenPossible" on downloads that end up in that directory, so we may just keep the "getTemporaryDownloadsDirectory" name, in that it is for "temporary downloads" despite it's not necessarily the system's temporary directory.
(Assignee)

Comment 5

5 years ago
Created attachment 808952 [details] [diff] [review]
Patch for checkin

(In reply to :Paolo Amadini from comment #3)
> Comment on attachment 807033 [details] [diff] [review]
> Change getUserDownloadsDirectory to getPreferredDownloadsDirectory
> 
> Review of attachment 807033 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/jsdownloads/test/unit/test_DownloadIntegration.js
> @@ +192,1 @@
> >      do_check_eq(downloadDir.path, userDownloadDir.path);
> 
> nit: "preferredDownloadDir"

Addressed.


Pushed to try and waiting for result
https://tbpl.mozilla.org/?tree=Try&rev=63b8608acb86
Assignee: nobody → raymond
Attachment #807033 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Comment on attachment 808952 [details] [diff] [review]
Patch for checkin

In the future, please just use checkin-needed for single-patch bugs :)
Attachment #808952 - Flags: checkin? → checkin+
https://hg.mozilla.org/integration/fx-team/rev/26812d8d7a6d
Flags: in-testsuite+
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/26812d8d7a6d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla27
(Assignee)

Updated

5 years ago
Summary: Rename getUserDownloadsDirectory and getTemporaryDownloadsDirectory → Rename getUserDownloadsDirectory
You need to log in before you can comment on or make changes to this bug.