1835419 - Last "Save As" location from Private mode surfaces when saving PDF after closing Private mode

Status: NEW

(Toolkit :: Printing, defect)

Description

[CB]

Steps to reproduce:

In Windows (not sure about Mac OS), Open Private window, save image to non-default file location, close private window, open regular window, print something, save as PDF through print dialogue, and then the file location for saving will be the same as the private window session that was closed.

Actual results:

When saving the PDF file, the initial file explorer location was from the private window session that was closed.

Expected results:

When saving PDF files outside of a private session, the file explorer location should be the default download location

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::Printing: Output' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

save image to non-default file location

Gijs do you happen to know where this is persisted / if it's persisted on our end or on Windows'?

Comment 3

[:Gijs (away/busy until Mar 10)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

save image to non-default file location

Gijs do you happen to know where this is persisted / if it's persisted on our end or on Windows'?

I think all the code for this lives in https://searchfox.org/mozilla-central/source/toolkit/mozapps/downloads/DownloadLastDir.sys.mjs .

This bug is sort of surprising because the doc comment at the top suggests that the behaviour outlined in comment 0 should not be possible, as private browsing information is only stored in memory. And indeed, you can see it's cleared out when the last private window closes. I don't know where the dir would be coming from for the save-to-pdf print bits. It looks like it might be here, and that doesn't talk to the last dir stuff (doesn't seem like it's telling the picker where to open at all!), so I imagine this is coming from the OS.

But I only repro'd on macOS, and it seems like a peculiar coincidence for Windows and macOS to both persist this the same way independently. OTOH, from an OS perspective it's probably not unreasonable to assume that the second filepicker for a given app should restart in the same place, without work from the app. I think it'd go away if we explicitly told the picker where to open in the print-to-pdf code. Emilio, does that sound right?

If there is OS persistence here, it would be interesting to see if there's some OS flag we can use to tell it not to store it anywhere, as I imagine that the OS persists it... somewhere... where ideally it should also not persist once the last private context closes.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Yep, that's the right place. It seems windows does persist it globally (regardless of PBM) here, though I don't see similar issues.

I don't see anything similar in macOS tho.

Comment 5

[Mark Striemer [:mstriemer]]

Opening our regular save dialog seems to avoid the bug, so:

1. Save image in Private Browsing
2. Close Private Browsing
3. Cmd+S/Ctrl+S, Escape

No longer reproduces. Not a great workaround but it does exist.

I don't see any print specific code causing this, so maybe we're missing something?

Comment 6

[:Gijs (away/busy until Mar 10)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

Yep, that's the right place. It seems windows does persist it globally (regardless of PBM) here, though I don't see similar issues.

I don't see anything similar in macOS tho.

Looks like that code got added for Windows 98 (yes, really) and Win2k does it for us anyway: https://bugzilla.mozilla.org/show_bug.cgi?id=120931#c5 .

I imagine we can get rid of it.

(bonus aside: spotted a 4 digit bug in hg annotate! Bug 9449 which made this unicode rather than ascii for memo-izing the last used dir...)

(In reply to Mark Striemer [:mstriemer] from comment #5)

Opening our regular save dialog seems to avoid the bug, so:

1. Save image in Private Browsing
2. Close Private Browsing
3. Cmd+S/Ctrl+S, Escape

No longer reproduces. Not a great workaround but it does exist.

I don't see any print specific code causing this, so maybe we're missing something?

I think the point is the bug is caused by omission rather than commission. Windows and macOS (and, apparently, our own win32 widget layer, for extra fun, per Emilio's comment) reuses the last used directory unless the consumer specifically tells it something else to use as the dir to start the picker in. The cmd/ctrl-s case specifically tells it something, cf. this code. The print code doesn't. If it used DownloadLastDir correctly, the problem would go away.

A more thorough fix might be requiring some kind of context object in filepicker.init that indicates private-ness and/or what dir to open, or something, so that people don't make this mistake in future and/or we flush out other places that don't pass something. It looks like we already require a loadContext, and https://searchfox.org/mozilla-central/rev/daedd554ae8a2c7f420ad77311134c8c298ba318/widget/windows/nsFilePicker.cpp#174 will avoid adding these files to "recent files" for private load contexts. The print code does already pass the parent window, which will have the right load context to make this work. I think if we pass OFN_NOCHANGEDIR in the flags value of the struct in that same private-browsing-specific block, that would fix the issue as filed (if we also removed our own manual memo-ization of the dir). It would still be good to also integrate the print code file picker with DownloadLastDir anyway.