Closed Bug 347210 Opened 18 years ago Closed 16 years ago

nsIBookmarksService needs createSeparatorInContainer

Categories

(Firefox :: Bookmarks & History, defect)

Product:
Firefox
Component:
Bookmarks & History
Version:
2.0 Branch
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: mkaply, Assigned: mkaply)

Details

Attachments

(2 files, 1 obsolete file)

Just the code
(deleted), patch
Details | Diff | Splinter Review
New patch
(deleted), patch
benjamin
: review+
beltzner
: approval1.8.1+
Details | Diff | Splinter Review
In using the bookmarks service with the CCK, there is no easy way to add separators given an RDF resource.
Attached patch Just the code (deleted) — — Splinter Review
I'll also need to do something with the uid and create another file I think?
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Attached patch branch diff (obsolete) (deleted) — — Splinter Review
This is a patch specifically for the 1.8 branch
Attachment #231992 - Flags: review?(benjamin)
Comment on attachment 231992 [details] [diff] [review] branch diff >Index: bookmarks/src/nsBookmarksService.cpp > NS_IMETHODIMP >+nsBookmarksService::CreateSeparatorInContainer(nsIRDFResource* aParentFolder, >+ PRInt32 aIndex, >+ nsIRDFResource** aResult) >+{ >+ nsCOMPtr<nsIRDFNode> nodeType; >+ GetSynthesizedType(aParentFolder, getter_AddRefs(nodeType)); >+ if (nodeType == kNC_Livemark) >+ return NS_RDF_ASSERTION_REJECTED; >+ >+ nsresult rv = CreateSeparator(aResult); >+ if (NS_SUCCEEDED(rv)) >+ rv = InsertResource(*aResult, aParentFolder, aIndex); >+ return rv; >+} If CreateSeparator succeeds but InsertResource fails, you'll end up leaking (or potentially leaking) aResult.
Attachment #231992 - Flags: review?(benjamin) → review-
(In reply to comment #3) > (From update of attachment 231992 [details] [diff] [review] [edit]) > If CreateSeparator succeeds but InsertResource fails, you'll end up leaking (or > potentially leaking) aResult. > Interesting. I copied the code from elsewhere in the file: http://lxr.mozilla.org/seamonkey/source/browser/components/bookmarks/src/nsBookmarksService.cpp#2943 http://lxr.mozilla.org/seamonkey/source/browser/components/bookmarks/src/nsBookmarksService.cpp#2828 So that bug exists elsewhere as well. Would I just NS_RELEASE on the RDFResource?
Attached patch New patch (deleted) — — Splinter Review
This patch releases upon failure, and fixes the other places in this file that have that problem as well.
Attachment #231992 - Attachment is obsolete: true
Attachment #232003 - Flags: review?(benjamin)
Attachment #232003 - Flags: review?(benjamin) → review+
Comment on attachment 232003 [details] [diff] [review] New patch This is a branch only patch that adds a new interface that allows the CCK to create separators. Needed for some of the custom builds that are being done by MoCo.
Attachment #232003 - Flags: approval1.8.1?
Comment on attachment 232003 [details] [diff] [review] New patch a=drivers, please land on the branch.
Attachment #232003 - Flags: approval1.8.1? → approval1.8.1+
Seems like you should at least get the leak fixes into the trunk, if not the whole thing.
This will definitely go on the trunk as well in a little different incarnation.
Possible regression: BeOS branch build broke sometime between 2006-07-30 and today.
Updated from CVS again - just picked up revised patch and can build now. Apologies for bug-chatter.
Version: Trunk → 2.0 Branch
This code is not in the trunk anymore, so this was 1.8 only.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: