Closed Bug 917217 Opened 6 years ago Closed 6 years ago

Rename getUserDownloadsDirectory

Categories

(Toolkit :: Downloads API, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: Paolo, Assigned: raymondlee)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Attachment #807033 - Flags: review?(paolo.mozmail)
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+
(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.
(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
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla27
Summary: Rename getUserDownloadsDirectory and getTemporaryDownloadsDirectory → Rename getUserDownloadsDirectory
You need to log in before you can comment on or make changes to this bug.