298429 - Opening Cache preferences page adds cache pref

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Frank Wein [:mcsmurf]]

When you open the preferences window and select Advanced->Cache, the preference
browser.cache.disk.parent_directory will get written. This pref is no longer
needed normally (except you want to change the path to the cache, of course ;).

Comment 1

[Frank Wein [:mcsmurf]]

Attachment: Patch

Comment 2

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

Comment on attachment 187004 [details] [diff] [review]
Patch

why remove the try..catch?

Comment 3

[Frank Wein [:mcsmurf]]

Oh, was the try...catch because of the var dirSvc =
Components.classes["@mozilla.org/file/directory_service;1"].getService(nsIProperties);?
If yes i can add it back, but if that fails we're screwed anyway IMHO :).

Comment 4

[Frank Wein [:mcsmurf]]

Oh, was the try...catch because of the var dirSvc =
Components.classes["@mozilla.org/file/directory_service;1"].getService(nsIProperties);?
If yes i can add it back, but if that fails we're screwed anyway IMHO :).

Comment 5

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

Comment on attachment 187004 [details] [diff] [review]
Patch

I thought it was also for the .get... please add it back though

Comment 6

[Frank Wein [:mcsmurf]]

Attachment: Patch 2

Comment 7

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

Comment on attachment 187121 [details] [diff] [review]
Patch 2

r=me, although I would prefer if the code stored the file from the
filepicker/pref instead of recreating it.

Comment 8

[[e-mail]]

Comment on attachment 187121 [details] [diff] [review]
Patch 2

Recreating the file won't work on the Mac.

Comment 9

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

oops. yeah. I suck, I forgot that this dialog was one of them...

Comment 10

[Frank Wein [:mcsmurf]]

Attachment: Patch 3

Comment 11

[[e-mail]]

Comment on attachment 187292 [details] [diff] [review]
Patch 3

> var gFolderField;
>+var dir;
This should be called something more meaningful e.g. gCacheParentDir; sr=me
with this fixed.

>     var localFile = fp.file.QueryInterface(nsILocalFile);
I wonder whether you ought to save this as the default in case the browse
button is clicked again (particularly once someone manages to fix this not to
be instant-apply).

Comment 12

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

Comment on attachment 187292 [details] [diff] [review]
Patch 3

I wonder if we should put code somewhere to clear the pref if it is equal to
the default. (but this pref panel would be the wrong place for it)

I think that you should assign the new directory to your global variable, so
that clicking browse again does the right thing.

wanna file a bug on stopping doing the instant-apply thing?

Setting displayDirectory to null crashes, so don't do that (bug 252058)

Comment 13

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

Comment on attachment 187292 [details] [diff] [review]
Patch 3

ok, looks like passing null does not actually crash anymore

Comment 14

[timeless]

libpref already forgets any prefs that match defaults

Comment 15

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

this is not one of those obviously, because the default only lives in code, not
in all.js, and can't live there.

Comment 16

[Frank Wein [:mcsmurf]]

Comment on attachment 187292 [details] [diff] [review]
Patch 3

seamonkey only

Comment 17

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

Checking in xpfe/components/prefwindow/resources/content/pref-cache.js;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-cache.js,v 
<--  pref-cache.js
new revision: 1.18; previous revision: 1.17
done

Can someone file a bug about automatically removing that pref if it exists?

Comment 18

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

(and is equal to the former default location)

Comment 19

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

Attachment: patch to fix review comments

Checking in pref-cache.js;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-cache.js,v 
<--  pref-cache.js
new revision: 1.19; previous revision: 1.18

Comment 20

[Frank Wein [:mcsmurf]]

I filed Bug 301668 about the issue that the pref applies immediately not after
clicking OK. Will file another bug for the issue mentioned in Comment 17