80108 - deleteBranch method of nsIPrefBranch does not work when null or "" is passed.

Status: RESOLVED FIXED

(Core :: Preferences: Backend, defect, P3)

Description

[dsirnapalli]

I used the piece of code below to test deleteBranch method.
var nsIPrefBranchObj = nsIPrefServiceObj.getBranch("font.");
nsIPrefBranchObj.deleteBranch(null); 
nsIPrefServiceObj.savePrefFile(null);

Since the root is set "font." and null is passed to deleteBranch method,all of 
the preferences related to "font" such as "font.name.sans-serif.x-western", 
"font.size.fixed.x-western" and "font.size.variable.x-western" should be 
removed, but is not removing. Even if i use deleteBranch("") its not removing.
it works if i pass a string like deleteBranch("size").

Comment 1

[Brian Nesse (gone)]

This is because the old 4.x pref code (which prefBranch is calling) is adding a
"." to whatever is passed in. This will work if you create the service using ""
and pass "font" in to the DeleteBranch call. The underlying code needs to be
fixed so it only appends the "." if the last character isn't already a "."

Comment 2

[Brian Nesse (gone)]

Attachment: Patch to eliminate double addition of "." to branchname.

Comment 3

[Conrad Carlen (not reading bugmail)]

Since this line:

char* branch_dot = PR_smprintf("%s.", branch_name);

appends a dot onto branch_name and returns the result, wouldn't you want to
check for the dot first and then not append (which alocates memory) unless you
had to. It seems that unconditionally appending the dot and then trimming it is
wasteful.

Comment 4

[Brian Nesse (gone)]

I agree it's wasteful, but I was trying not to rearchitect the whole function.
I'll work up a patch that works the other way for comparison.

Comment 5

[Brian Nesse (gone)]

Attachment: Revised patch which only allocates memory when needed.

Comment 6

[Conrad Carlen (not reading bugmail)]

That's good. Man, moral of the story is: if this code was C++ instead of C and
we could use nifty nsStrings and string iterators, life would be better.
r=ccarlen

Comment 7

[Brian Nesse (gone)]

Someday maybe, but not today. :)

Comment 8

[Alec Flett]

good god :)
sr=alecf

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

a=dbaron for trunk checkin (on behalf of drivers)

Comment 10

[Brian Nesse (gone)]

Fix checked in.