Closed Bug 210541 Opened 21 years ago Closed 21 years ago

Bookmarks now crash if nsIProfile impl is not present.

Categories

(SeaMonkey :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ccarlen, Assigned: ccarlen)

References

Details

(Keywords: crash, fixed1.4.1, regression)

Attachments

(1 file)

Notice that here:
http://lxr.mozilla.org/mozilla/source/xpfe/components/bookmarks/src/nsBookmarksService.cpp#1786,
there's no null-check. If the profile mgr is not present (and it's not in many
embedding apps) this will crash.
Keywords: crash, regression
is there a patch coming? I'll review :)
Attached patch patchSplinter Review
The patch is -w, since I had to indent a sizable block.
Attachment #126389 - Flags: superreview?(alecf)
Attachment #126389 - Flags: review?(varga)
Comment on attachment 126389 [details] [diff] [review]
patch

Eek! the 1.4 branch has this problem. Requesting a=.
Attachment #126389 - Flags: approval1.4?
Comment on attachment 126389 [details] [diff] [review]
patch

I'm sure you've already confirmed this just by the nature of this patch, but I
just want to double-check that
1) rv isn't uninitialized  now, or has some bogus value after the first hunk of
the patch
2) that the other people who use useProfile aren't getting it in some wierd
success or error condition now

r/sr=alecf assuming that's all cool - this is a null (success) check, mostly
low risk.. don't delay checking this into 1.4 in case you can't get a review
from Jan.
Attachment #126389 - Flags: superreview?(alecf) → superreview+
if you want r=timeless you can have it, i talked someone through this problem
when they foolishly built mozilla w/ the noprofiles flag.
Assignee: chanial → ccarlen
I am guilty of this left-over (bug 36339)
conrad's patch is ok with alec's comments: I simply missed the check for rv, and
this nsIprofile service result is not used (and is overwritten after).
In addition, useprofile is not used after the statement that simply sets the
bookmarks top root name as 'Bookmarks'.

thanks!
r=pch
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment on attachment 126389 [details] [diff] [review]
patch

moving approval request forward.
Attachment #126389 - Flags: approval1.4? → approval1.4.x?
Attachment #126389 - Flags: review?(varga) → review+
Comment on attachment 126389 [details] [diff] [review]
patch

a=mkaply
Attachment #126389 - Flags: approval1.4.x? → approval1.4.x+
Please add the fixed1.4.1 keyword when this is checked in.
Flags: blocking1.4.x+
checked into the 1.4 branch
Keywords: fixed1.4.1
Blocks: 224532
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: