Open Bug 847949 Opened 7 years ago Updated 6 months ago

DownloadURL should get the most recent window of the same privacy status as the current one

Categories

(Firefox :: Downloads Panel, defect)

x86_64
Linux
defect
Not set

Tracking

()

REOPENED

People

(Reporter: jdm, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(1 file)

>51 function DownloadURL(aURL, aFileName) {
>52   // For private browsing, try to get document out of the most recent browser
>53   // window, or provide our own if there's no browser window.
>54   let browserWin = RecentWindow.getMostRecentBrowserWindow();
>55   let initiatingDoc = browserWin ? browserWin.document : document;
>56   saveURL(aURL, aFileName, null, true, true, undefined, initiatingDoc);

This doesn't look right to me. I think we want to be getting the most recent browser window of the same privacy status instead.
Assignee: nobody → josh
Comment on attachment 722262 [details] [diff] [review]
Retrieve most recent window of same privacy status when initiating a new download via the panel.

So, this fixes the case where URLs pasted or dropped in the Library, or
downloads restarted from the Library, would become private if the most
recently focused browser window was private. Is this correct?

I'd update the bug title to make this clear, as well as the patch title that
references the panle though this only affects the Library window and the
in-content Downloads view instead.
Attachment #722262 - Flags: review?(paolo.mozmail) → review+
By the way... if the aSourceDocument parameter of saveURL is only ever used for
getting the privacy status of the window, and is also already supposed to work
correctly if we pass a reference to the Library window's document or to a content
document loaded in a private window, can't we just do like this instead of going
through hoops?

saveURL(aURL, aFileName, null, true, true, undefined, document);

Marco, is there some special case I'm missing here?

Or I suppose we could do this is we need to get the top-level chrome document
from the in-content view:

https://developer.mozilla.org/en-US/docs/Working_with_windows_in_chrome_code#Accessing_the_elements_of_the_top-level_document_from_a_child_window
Flags: needinfo?(mak77)
(In reply to Paolo Amadini [:paolo] from comment #3)
> By the way... if the aSourceDocument parameter of saveURL is only ever used
> for
> getting the privacy status of the window, and is also already supposed to
> work
> correctly if we pass a reference to the Library window's document or to a
> content
> document loaded in a private window, can't we just do like this instead of
> going
> through hoops?

Yeah, I had the same question. Why not always use the panel document?
I think there's a bit of confusion here, this code is about the Library or in-content view, it has nothing to do with the panel, so it cannot just use document to figure privacy status when it's in the Library, since the Library window is never private.

I'm not sure what's the expected behavior here, the current implementation guesses the wanted behavior based on the most recent window, that was done to minimize the possible privacy hits of a user opening the Library from a private window and dropping links to the Library or pasting them, to start a new download.

After this patch we'd instead always begin new downloads in a non-private way if the user drops/paste in the Library, and always in a private way if he instead drops/paste to the in-content downloads view.

This would clearly become a nonissue once we move the Library in content, since then we could just rely on document.

If the patch behavior is what we want (I honestly find it odd, but there is no perfect solution until Library moves to a tab), what paolo suggests in comment 3 is basically the same as the current patch, since if document is the Library we'll begin non-private dl, otherwise we'll begin private dl.
Flags: needinfo?(mak77)
Thanks for the clear explanation. This sounds like wontfix to me; I misunderstood the context in which the code was run.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
(In reply to Marco Bonardo [:mak] from comment #5)
> I'm not sure what's the expected behavior here, the current implementation
> guesses the wanted behavior based on the most recent window, that was done
> to minimize the possible privacy hits of a user opening the Library from a
> private window and dropping links to the Library or pasting them, to start a
> new download.

I share the "not sure" about paste behavior, the fact that the Library window
has erratic rather than consistent behavior here could be a more severe privacy
concern - whether the download is private would depend on which window you
focused most recently, that may not be obvious to remember after some time passes.

Also, considering that the Library window only ever shows public downloads, if
you start a private download by pasting a link, it won't appear in the window,
effectively giving no feedback unless the private browsing window is also visible.

In any case, there is still the issue that DownloadURL is also used to restart
history downloads, meaning that a public download in the Library window could
possibly be restarted incorrectly as a private one. So, even if we really want
pasted downloads to be possibly private, we should at least use a different
function for restarting public history downloads.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Also, the "privacy" of a download is not about not having data saved to disk, but
about not leaving traces in history; pasting in the history window makes it very
visible that you're leaving traces, and you could easily change your mind by
removing the download from the window.

About the cookies used for the request, I think they're less likely to be important
when you paste URLs from an external source, compared to when you follow a link.
Assignee: josh → nobody
You need to log in before you can comment on or make changes to this bug.