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

RESOLVED FIXED in mozilla0.9.2

Status

()

P3
normal
RESOLVED FIXED
18 years ago
18 years ago

People

(Reporter: dsirnapalli, Assigned: bnesse)

Tracking

Trunk
mozilla0.9.2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [r][sr][a])

Attachments

(2 attachments)

(Reporter)

Description

18 years ago
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").
(Assignee)

Comment 1

18 years ago
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 "."
Status: NEW → ASSIGNED
Component: Embedding APIs → Preferences: Backend
OS: Windows NT → All
Priority: -- → P3
Hardware: PC → All
Target Milestone: --- → mozilla0.9.2
(Assignee)

Comment 2

18 years ago
Created attachment 37398 [details] [diff] [review]
Patch to eliminate double addition of "." to branchname.
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.
(Assignee)

Comment 4

18 years ago
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.
(Assignee)

Comment 5

18 years ago
Created attachment 37410 [details] [diff] [review]
Revised patch which only allocates memory when needed.
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
(Assignee)

Comment 7

18 years ago
Someday maybe, but not today. :)

Comment 8

18 years ago
good god :)
sr=alecf
(Assignee)

Updated

18 years ago
Whiteboard: [r][sr][need drivers approval]

Updated

18 years ago
Blocks: 83989
a=dbaron for trunk checkin (on behalf of drivers)
Whiteboard: [r][sr][need drivers approval] → [r][sr][a]
(Assignee)

Comment 10

18 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Updated

18 years ago
QA Contact: mdunn → dsirnapalli
You need to log in before you can comment on or make changes to this bug.