318048 - viewing source in external editor never deletes the temp. file

Status: RESOLVED FIXED

(Toolkit :: View Source, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

from bug 172817

 ------- Comment #127 From Jason Barnabe (np)  2005-11-28 14:15 PST  [reply] -------

(In reply to comment #126)
> am I reading this right that the temporary files are never deleted?
Never deleted by us. Since it's in the system temp directory, the OS will
delete them whenever it sees fit.

[...]

------- Comment #128 From Christian Biesinger (:bi) (reviews slower than usual) 2005-11-28 14:19 PST [reply] -------

what if the OS doesn't? (e.g. FC4, Windows)


------- Comment #129 From Jason Barnabe (np) 2005-11-28 14:48 PST [reply] -------

(In reply to comment #128)
> what if the OS doesn't?
Then the file will stay there forever. If you think this is a problem, feel
free to file a new bug.

Comment 1

[Jason Barnabe (np)]

FMI, what do you feel is the problem with leaving the temp files? Privacy concerns? Hard drive space?

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

privacy + cluttered up /tmp directory (still annoyed that fc4 doesn't kill it on startup). and of course the general principle that apps should clean up their temp data.

Comment 3

[Jason Barnabe (np)]

Attachment: patch [Checkin: Comment 8]

nsPIExternalAppLauncher seems to expect the file to exist when you register it. (Maybe that should be documented?) Putting the new code in getTemporaryFile made the file deletion hit or miss, so I moved the code to after we're done writing to the file.

Comment 4

[Asaf Romano (gone)]

Comment on attachment 211498 [details] [diff] [review]
patch
[Checkin: Comment 8]

Moving the request over to gavin since I cannot test this on mac, sorry for the ver long delay here.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 211498 [details] [diff] [review]
patch
[Checkin: Comment 8]

r=me, but it might be worth filing a bug about documenting the restriction that deleteTemporaryFileOnExit's argument be an existing file, or removing it. It would also be good to have a bug filed on http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/uriloader/exthandler/nsIExternalHelperAppService.idl&rev=1.24&mark=90-92#89 .

Comment 6

[Matthias Versen [:Matti]]

biesi: Did you forgot this patch ?

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

it's not my patch...

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 211498 [details] [diff] [review]
patch
[Checkin: Comment 8]

I manually "applied" it to current Hg tree.

http://hg.mozilla.org/mozilla-central/rev/7a7bb7cbbc3d