From c5792178488b220d2ae21d2b827b367db3e00084 Mon Sep 17 00:00:00 2001 From: "brettw%gmail.com" Date: Thu, 13 Apr 2006 16:35:49 +0000 Subject: [PATCH] Bug 332143 r=brettw sr=beng (checkin for pamg) New livemarks don't get proper icon in toolbar. --- .../places/public/nsINavBookmarksService.idl | 6 +-- .../places/src/nsLivemarkService.cpp | 4 +- .../components/places/src/nsNavBookmarks.cpp | 45 ++++++++++++++----- .../components/places/src/nsNavBookmarks.h | 27 +++++++---- 4 files changed, 57 insertions(+), 25 deletions(-) diff --git a/browser/components/places/public/nsINavBookmarksService.idl b/browser/components/places/public/nsINavBookmarksService.idl index 2f3a95839da..3e98dfdafe2 100644 --- a/browser/components/places/public/nsINavBookmarksService.idl +++ b/browser/components/places/public/nsINavBookmarksService.idl @@ -200,7 +200,7 @@ interface nsINavBookmarkObserver : nsISupports * folders. A URI in history can be contained in one or more such folders. */ -[scriptable, uuid(df93900d-7ef1-4c5f-8e44-a930aeaf1462)] +[scriptable, uuid(860d786d-9bba-4011-a396-486a87af8f07)] interface nsINavBookmarksService : nsISupports { /** @@ -264,12 +264,12 @@ interface nsINavBookmarksService : nsISupports * parent and sets the container type. * @param parent The id of the parent folder * @param name The name of the new folder - * @param index The index to insert at, or -1 to append * @param type The type of container to insert + * @param index The index to insert at, or -1 to append * @returns the ID of the newly-inserted folder */ PRInt64 createContainer(in PRInt64 parent, in AString name, - in PRInt32 index, in AString type); + in AString type, in PRInt32 index); /** * Removes a folder from the bookmarks tree. diff --git a/browser/components/places/src/nsLivemarkService.cpp b/browser/components/places/src/nsLivemarkService.cpp index 99502c4700d..b575aaba213 100755 --- a/browser/components/places/src/nsLivemarkService.cpp +++ b/browser/components/places/src/nsLivemarkService.cpp @@ -260,9 +260,9 @@ nsLivemarkService::CreateLivemark(PRInt64 aFolder, { // Create the livemark as a bookmark container nsNavBookmarks *bookmarks = nsNavBookmarks::GetBookmarksService(); - nsresult rv = bookmarks->CreateContainer(aFolder, aName, aIndex, + nsresult rv = bookmarks->CreateContainer(aFolder, aName, NS_LITERAL_STRING(NS_LIVEMARKSERVICE_CONTRACTID), - aNewLivemark); + aIndex, aNewLivemark); NS_ENSURE_SUCCESS(rv, rv); // Get the livemark URI diff --git a/browser/components/places/src/nsNavBookmarks.cpp b/browser/components/places/src/nsNavBookmarks.cpp index 9c55aaf19fb..41dfe347639 100644 --- a/browser/components/places/src/nsNavBookmarks.cpp +++ b/browser/components/places/src/nsNavBookmarks.cpp @@ -970,30 +970,36 @@ NS_IMETHODIMP nsNavBookmarks::CreateFolder(PRInt64 aParent, const nsAString &aName, PRInt32 aIndex, PRInt64 *aNewFolder) { - return CreateFolderWithID(-1, aParent, aName, aIndex, aNewFolder); + // CreateFolderWithID returns the index of the new folder, but that's not + // used here. To avoid any risk of corrupting data should this function + // be changed, we'll use a local variable to hold it. The PR_TRUE argument + // will cause notifications to be sent to bookmark observers. + PRInt32 localIndex = aIndex; + return CreateFolderWithID(-1, aParent, aName, PR_TRUE, &localIndex, aNewFolder); } NS_IMETHODIMP nsNavBookmarks::CreateContainer(PRInt64 aParent, const nsAString &aName, - PRInt32 aIndex, const nsAString &aType, + const nsAString &aType, PRInt32 aIndex, PRInt64 *aNewFolder) { - return CreateContainerWithID(-1, aParent, aName, aIndex, aType, aNewFolder); + return CreateContainerWithID(-1, aParent, aName, aType, aIndex, aNewFolder); } nsresult nsNavBookmarks::CreateFolderWithID(PRInt64 aFolder, PRInt64 aParent, - const nsAString& aName, PRInt32 aIndex, - PRInt64* aNewFolder) + const nsAString& aName, + PRBool aSendNotifications, + PRInt32* aIndex, PRInt64* aNewFolder) { // You can pass -1 to indicate append, but no other negative number is allowed - if (aIndex < -1) + if (*aIndex < -1) return NS_ERROR_INVALID_ARG; mozIStorageConnection *dbConn = DBConn(); mozStorageTransaction transaction(dbConn, PR_FALSE); - PRInt32 index = (aIndex == -1) ? FolderCount(aParent) : aIndex; + PRInt32 index = (*aIndex == -1) ? FolderCount(aParent) : *aIndex; nsresult rv = AdjustIndices(aParent, index, PR_INT32_MAX, 1); NS_ENSURE_SUCCESS(rv, rv); @@ -1040,20 +1046,31 @@ nsNavBookmarks::CreateFolderWithID(PRInt64 aFolder, PRInt64 aParent, rv = transaction.Commit(); NS_ENSURE_SUCCESS(rv, rv); - ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver, - OnFolderAdded(child, aParent, index)) + // When creating a livemark container, we need to delay sending notifications + // until the container type has been set. In that case, they'll be sent by + // CreateContainerWithID rather than here. + if (aSendNotifications) { + ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver, + OnFolderAdded(child, aParent, index)) + } + *aIndex = index; *aNewFolder = child; return NS_OK; } nsresult nsNavBookmarks::CreateContainerWithID(PRInt64 aFolder, PRInt64 aParent, - const nsAString &aName, PRInt32 aIndex, - const nsAString &aType, PRInt64 *aNewFolder) + const nsAString &aName, const nsAString &aType, + PRInt32 aIndex, PRInt64 *aNewFolder) { // Containers are wrappers around read-only folders, with a specific type. - nsresult rv = CreateFolderWithID(aFolder, aParent, aName, aIndex, aNewFolder); + // CreateFolderWithID will return the index of the newly created folder, + // which we will need later on in order to send notifications. The PR_FALSE + // argument disables sending notifiactions, since we need to defer that until + // the folder type has been set. + PRInt32 localIndex = aIndex; + nsresult rv = CreateFolderWithID(aFolder, aParent, aName, PR_FALSE, &localIndex, aNewFolder); NS_ENSURE_SUCCESS(rv, rv); // Set the type. @@ -1070,6 +1087,10 @@ nsNavBookmarks::CreateContainerWithID(PRInt64 aFolder, PRInt64 aParent, rv = statement->Execute(); NS_ENSURE_SUCCESS(rv, rv); + // Send notifications after folder type has been set. + ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver, + OnFolderAdded(*aNewFolder, aParent, localIndex)) + return NS_OK; } diff --git a/browser/components/places/src/nsNavBookmarks.h b/browser/components/places/src/nsNavBookmarks.h index cb3f864b200..0ca9e33a676 100644 --- a/browser/components/places/src/nsNavBookmarks.h +++ b/browser/components/places/src/nsNavBookmarks.h @@ -82,13 +82,22 @@ public: nsNavHistoryQueryOptions *aOptions, nsCOMArray *children); - // If aFolder is -1, use the autoincrement id for folder index. + // If aFolder is -1, uses the autoincrement id for folder index. Returns + // the index of the new folder in aIndex, whether it was passed in or + // generated by autoincrement. If aSendNotifications is true, sends + // OnFolderAdded notifications to bookmark observers. nsresult CreateFolderWithID(PRInt64 aFolder, PRInt64 aParent, - const nsAString& title, PRInt32 aIndex, - PRInt64* aNewFolder); - nsresult CreateContainerWithID(PRInt64 aFolder, PRInt64 aParent, - const nsAString& title, PRInt32 aIndex, - const nsAString& type, PRInt64* aNewFolder); + const nsAString& title, + PRBool aSendNotifications, + PRInt32 *aIndex, PRInt64* aNewFolder); + + // Creates a new container of the given type. If aFolder is -1, uses the + // autoincrement id for folder index. Sends OnFolderAdded notifications + // to all observers after the folder has been created and its type has + // been set. + nsresult CreateContainerWithID(PRInt64 aFolder, PRInt64 aParent, + const nsAString& title, const nsAString& type, + PRInt32 aIndex, PRInt64* aNewFolder); // Returns a statement to get information about a folder id mozIStorageStatement* DBGetFolderInfo() { return mDBGetFolderInfo; } @@ -197,10 +206,12 @@ private: NS_IMETHOD UndoTransaction() { nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService(); PRInt64 newFolder; + // If the transaction has no specific type, default to a folder, and send notifications + // to all bookmark observers (controlled by the PR_TRUE argument to CreateFolderWithID). if (mType.IsEmpty()) - return bookmarks->CreateFolderWithID(mID, mParent, mTitle, mIndex, &newFolder); + return bookmarks->CreateFolderWithID(mID, mParent, mTitle, PR_TRUE, &mIndex, &newFolder); nsAutoString type; type.AssignWithConversion(mType); - return bookmarks->CreateContainerWithID(mID, mParent, mTitle, mIndex, type, &newFolder); + return bookmarks->CreateContainerWithID(mID, mParent, mTitle, type, mIndex, &newFolder); } NS_IMETHOD RedoTransaction() {