460689 - Temporary files on OS X are no longer deleted on shutdown with browser.helperApps.deleteTempFileonExit set to true

Status: RESOLVED INVALID

(Firefox :: File Handling, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081019 Minefield/3.1b2pre

With the patch on bug 302433 we have a pref (browser.helperApps.deleteTempFileonExit) which controls if temporary files will be deleted on shutdown of Firefox. Lately I've noticed that this will not happen anymore with a trunk build.

Steps:
1. Open about:config, create a new boolean value with the name browser.helperApps.deleteTempFileonExit and give it the value true
2. Search on Google for a PDF file
3. Open this PDF file with an external helper application
4. Close the helper application
5. Close Firefox

As you can see the temporary file which was downloaded to the Desktop doesn't get removed.

We have to check, when this regressed. I'll try to get the regression range in the next days.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Unless we added a new place where we just totally fail to check this pref, that would mean that deletion is also broken on Linux and Windows, no?

If so, I'll see what I can do about getting a Linux regression range tomorrow.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This bug is accidentally caused by bug 395144 which changed the pref name to "browser.helperApps.deleteTempFileOnExit". Due to we differentiate between lowercase and uppercase characters we don't read the correct pref.

I'll come up with a patch shortly. Because of its simplicity and the underlying privacy issue it would be great to have it on the 1.9.0 branch.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Former pref name

So the question is what we really want to have. A random number of users will use the former pref name and will be affected by this bug. Is it worth by changing the case of only one letter?

Comment 4

[(no longer active)]

Henrik: thanks for your work on this.  I assume that bug 460609 is related as well?  Can you test and see if this patch fixes that bug as well, so that we can reliably dupe it?  Thanks!

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #4)
> Henrik: thanks for your work on this.  I assume that bug 460609 is related as
> well?  Can you test and see if this patch fixes that bug as well, so that we
> can reliably dupe it?  Thanks!

Ehsan, IMO this fix is required for further work on bug 460609. Just think about having the browser open all the time. If you leave the Private Browsing mode you do not necessarily shutdown the browser. If you are working on a public computer you don't want to see files left on disk.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Er... the core pref was always "browser.helperApps.deleteTempFileOnExit".  Bug 395144 just copy/pasted some code that read the same pref as far as I can tell...

Comment 7

[Boris Zbarsky [:bzbarsky]]

In other words, I don't see that deleteTempFileonExit (lowercase 'o') ever worked.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Boris, you are right. Sometimes it's better to stop working after 1 am. No idea why a lowercase 'o' was in my pref name. I know it worked months ago. So it was probably an accidentally change which was forgotten to revert. Sorry for the noise. I'll have a deeper look again this evening. So just let this bug open until then.

Comment 9

[(no longer active)]

(In reply to comment #5)
> Ehsan, IMO this fix is required for further work on bug 460609. Just think
> about having the browser open all the time. If you leave the Private Browsing
> mode you do not necessarily shutdown the browser. If you are working on a
> public computer you don't want to see files left on disk.

What I meant was, could this be the same bug?  In other words, if you perform the same STR outside the private browsing mode (and even with a build without the private browsing patch), does the same bug still exist?

I need to check if it's something caused by the PB patch or not...

Comment 10

[(no longer active)]

Henrik: are you able to reproduce this with the correct pref name set in your profile?

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I re-tested this with the correct pref name and it works seamlessly. Sorry for the confusion with the lower case character. Marking it as invalid.