Bugzilla

(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](https://searchfox.org/mozilla-central/rev/daedd554ae8a2c7f420ad77311134c8c298ba318/widget/windows/nsFilePicker.cpp#35,304), 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](https://searchfox.org/mozilla-central/rev/daedd554ae8a2c7f420ad77311134c8c298ba318/toolkit/content/contentAreaUtils.js#699). 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, 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.

(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](https://searchfox.org/mozilla-central/rev/daedd554ae8a2c7f420ad77311134c8c298ba318/widget/windows/nsFilePicker.cpp#35,304), 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](https://searchfox.org/mozilla-central/rev/daedd554ae8a2c7f420ad77311134c8c298ba318/toolkit/content/contentAreaUtils.js#699). 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.