Closed
Bug 347210
Opened 18 years ago
Closed 16 years ago
nsIBookmarksService needs createSeparatorInContainer
Categories
(Firefox :: Bookmarks & History, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: mkaply, Assigned: mkaply)
Details
Attachments
(2 files, 1 obsolete file)
1.92 KB,
patch
|
Details | Diff | Splinter Review | |
4.24 KB,
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.
Assignee | ||
Comment 1•18 years ago
|
||
I'll also need to do something with the uid and create another file I think?
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•18 years ago
|
||
This is a patch specifically for the 1.8 branch
Attachment #231992 -
Flags: review?(benjamin)
Comment 3•18 years ago
|
||
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-
Assignee | ||
Comment 4•18 years ago
|
||
(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?
Assignee | ||
Comment 5•18 years ago
|
||
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)
Updated•18 years ago
|
Attachment #232003 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 6•18 years ago
|
||
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 7•18 years ago
|
||
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.
Assignee | ||
Comment 9•18 years ago
|
||
This will definitely go on the trunk as well in a little different incarnation.
Comment 10•18 years ago
|
||
Possible regression: BeOS branch build broke sometime between 2006-07-30 and today.
Comment 11•18 years ago
|
||
Updated from CVS again - just picked up revised patch and can build now. Apologies for bug-chatter.
Updated•16 years ago
|
Version: Trunk → 2.0 Branch
Assignee | ||
Comment 12•16 years ago
|
||
This code is not in the trunk anymore, so this was 1.8 only.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Keywords: dev-doc-needed
Updated•15 years ago
|
Keywords: dev-doc-needed
You need to log in
before you can comment on or make changes to this bug.
Description
•