diff --git a/toolkit/components/places/Helpers.cpp b/toolkit/components/places/Helpers.cpp index e0f70491bde..49ba9da6a52 100644 --- a/toolkit/components/places/Helpers.cpp +++ b/toolkit/components/places/Helpers.cpp @@ -352,6 +352,15 @@ IsValidGUID(const nsCString& aGUID) return true; } +void +TruncateTitle(const nsACString& aTitle, nsACString& aTrimmed) +{ + aTrimmed = aTitle; + if (aTitle.Length() > TITLE_LENGTH_MAX) { + aTrimmed = StringHead(aTitle, TITLE_LENGTH_MAX); + } +} + void ForceWALCheckpoint() { diff --git a/toolkit/components/places/Helpers.h b/toolkit/components/places/Helpers.h index 0c11b53aa33..cf6253fc9db 100644 --- a/toolkit/components/places/Helpers.h +++ b/toolkit/components/places/Helpers.h @@ -150,7 +150,7 @@ void GetReversedHostname(const nsString& aForward, nsString& aRevHost); * @param aInput * The string to be reversed * @param aReversed - * Ouput parameter will contain the reversed string + * Output parameter will contain the reversed string */ void ReverseString(const nsString& aInput, nsString& aReversed); @@ -170,6 +170,16 @@ nsresult GenerateGUID(nsCString& _guid); */ bool IsValidGUID(const nsCString& aGUID); +/** + * Truncates the title if it's longer than TITLE_LENGTH_MAX. + * + * @param aTitle + * The title to truncate (if necessary) + * @param aTrimmed + * Output parameter to return the trimmed string + */ +void TruncateTitle(const nsACString& aTitle, nsACString& aTrimmed); + /** * Used to finalize a statementCache on a specified thread. */ diff --git a/toolkit/components/places/nsINavBookmarksService.idl b/toolkit/components/places/nsINavBookmarksService.idl index d40c4b97f0d..44450ba1dc5 100644 --- a/toolkit/components/places/nsINavBookmarksService.idl +++ b/toolkit/components/places/nsINavBookmarksService.idl @@ -333,6 +333,9 @@ interface nsINavBookmarksService : nsISupports * @param aTitle * The title for the new bookmark * @return The ID of the newly-created bookmark. + * + * @note aTitle will be truncated to TITLE_LENGTH_MAX and + * aURI will be truncated to URI_LENGTH_MAX. */ long long insertBookmark(in long long aParentId, in nsIURI aURI, in long aIndex, in AUTF8String aTitle); @@ -471,9 +474,11 @@ interface nsINavBookmarksService : nsISupports /** * Set the title for an item. * @param aItemId - * The id of the item whose title should be updated + * The id of the item whose title should be updated. * @param aTitle * The new title for the bookmark. + * + * @note aTitle will be truncated to TITLE_LENGTH_MAX. */ void setItemTitle(in long long aItemId, in AUTF8String aTitle); diff --git a/toolkit/components/places/nsNavBookmarks.cpp b/toolkit/components/places/nsNavBookmarks.cpp index de2ceb6d0a4..864b9fccdb6 100644 --- a/toolkit/components/places/nsNavBookmarks.cpp +++ b/toolkit/components/places/nsNavBookmarks.cpp @@ -706,8 +706,11 @@ nsNavBookmarks::InsertBookmark(PRInt64 aFolder, *aNewBookmarkId = -1; PRTime dateAdded = PR_Now(); nsCAutoString guid; + nsCString title; + TruncateTitle(aTitle, title); + rv = InsertBookmarkInDB(placeId, BOOKMARK, aFolder, index, - aTitle, dateAdded, nsnull, EmptyString(), + title, dateAdded, nsnull, EmptyString(), aNewBookmarkId, guid); NS_ENSURE_SUCCESS(rv, rv); @@ -725,7 +728,7 @@ nsNavBookmarks::InsertBookmark(PRInt64 aFolder, NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers, nsINavBookmarkObserver, OnItemAdded(*aNewBookmarkId, aFolder, index, TYPE_BOOKMARK, - aURI, aTitle, dateAdded, guid, folderGuid)); + aURI, title, dateAdded, guid, folderGuid)); // If the bookmark has been added to a tag container, notify all // bookmark-folder result nodes which contain a bookmark for the new @@ -994,8 +997,11 @@ nsNavBookmarks::CreateContainerWithID(PRInt64 aItemId, nsCAutoString guid; ItemType containerType = aIsBookmarkFolder ? FOLDER : DYNAMIC_CONTAINER; + nsCString title; + TruncateTitle(aTitle, title); + rv = InsertBookmarkInDB(-1, containerType, aParent, index, - aTitle, dateAdded, nsnull, aContractId, aNewFolder, + title, dateAdded, nsnull, aContractId, aNewFolder, guid); NS_ENSURE_SUCCESS(rv, rv); @@ -1009,7 +1015,7 @@ nsNavBookmarks::CreateContainerWithID(PRInt64 aItemId, NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers, nsINavBookmarkObserver, OnItemAdded(*aNewFolder, aParent, index, containerType, - nsnull, aTitle, dateAdded, guid, folderGuid)); + nsnull, title, dateAdded, guid, folderGuid)); *aIndex = index; return NS_OK; @@ -1883,13 +1889,17 @@ nsNavBookmarks::SetItemTitle(PRInt64 aItemId, const nsACString& aTitle) NS_ENSURE_STATE(statement); mozStorageStatementScoper scoper(statement); + + nsCString title; + TruncateTitle(aTitle, title); + // Support setting a null title, we support this in insertBookmark. - if (aTitle.IsVoid()) { + if (title.IsVoid()) { rv = statement->BindNullByName(NS_LITERAL_CSTRING("item_title")); } else { rv = statement->BindUTF8StringByName(NS_LITERAL_CSTRING("item_title"), - aTitle); + title); } NS_ENSURE_SUCCESS(rv, rv); bookmark.lastModified = PR_Now(); @@ -1909,7 +1919,7 @@ nsNavBookmarks::SetItemTitle(PRInt64 aItemId, const nsACString& aTitle) OnItemChanged(bookmark.id, NS_LITERAL_CSTRING("title"), false, - aTitle, + title, bookmark.lastModified, bookmark.type, bookmark.parentId, diff --git a/toolkit/components/places/nsNavBookmarks.h b/toolkit/components/places/nsNavBookmarks.h index ed7728d8b87..2aea6b97ccf 100644 --- a/toolkit/components/places/nsNavBookmarks.h +++ b/toolkit/components/places/nsNavBookmarks.h @@ -211,9 +211,13 @@ public: PRInt64 aFolderId, mozIStoragePendingStatement** _pendingStmt); - // 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. + /** + * @return index of the new folder in aIndex, whether it was passed in or + * generated by autoincrement. + * + * @note If aFolder is -1, uses the autoincrement id for folder index. + * @note aTitle will be truncated to TITLE_LENGTH_MAX + */ nsresult CreateContainerWithID(PRInt64 aId, PRInt64 aParent, const nsACString& aTitle, const nsAString& aContractId, diff --git a/toolkit/components/places/tests/bookmarks/test_bookmarks.js b/toolkit/components/places/tests/bookmarks/test_bookmarks.js index b1e37c854e1..448dc9fd07b 100644 --- a/toolkit/components/places/tests/bookmarks/test_bookmarks.js +++ b/toolkit/components/places/tests/bookmarks/test_bookmarks.js @@ -56,6 +56,7 @@ let bookmarksObserver = { this._itemAddedParent = folder; this._itemAddedIndex = index; this._itemAddedURI = uri; + this._itemAddedTitle = title; // Ensure that we've created a guid for this item. let stmt = DBConn().createStatement( @@ -647,6 +648,23 @@ function run_test() { bs.insertBookmark(testRoot, uri1, bs.DEFAULT_INDEX, ""); hs.addVisit(uri1, Date.now() * 1000, null, hs.TRANSITION_TYPED, false, 0); + // bug 646993 - test bookmark titles longer than the maximum allowed length + let title15 = Array(TITLE_LENGTH_MAX + 5).join("X"); + let title15expected = title15.substring(0, TITLE_LENGTH_MAX); + let newId15 = bs.insertBookmark(testRoot, uri("http://evil.com/"), + bs.DEFAULT_INDEX, title15); + + do_check_eq(bs.getItemTitle(newId15).length, + title15expected.length); + do_check_eq(bookmarksObserver._itemAddedTitle, title15expected); + // test title length after updates + bs.setItemTitle(newId15, title15 + " updated"); + do_check_eq(bs.getItemTitle(newId15).length, + title15expected.length); + do_check_eq(bookmarksObserver._itemChangedId, newId15); + do_check_eq(bookmarksObserver._itemChangedProperty, "title"); + do_check_eq(bookmarksObserver._itemChangedValue, title15expected); + testSimpleFolderResult(); } @@ -688,13 +706,17 @@ function testSimpleFolderResult() { let folder = bs.createFolder(parent, "test folder", bs.DEFAULT_INDEX); bs.setItemTitle(folder, "test folder"); + let longName = Array(TITLE_LENGTH_MAX + 5).join("A"); + let folderLongName = bs.createFolder(parent, longName, bs.DEFAULT_INDEX); + do_check_eq(bookmarksObserver._itemAddedTitle, longName.substring(0, TITLE_LENGTH_MAX)); + let options = hs.getNewQueryOptions(); let query = hs.getNewQuery(); query.setFolders([parent], 1); let result = hs.executeQuery(query, options); let rootNode = result.root; rootNode.containerOpen = true; - do_check_eq(rootNode.childCount, 3); + do_check_eq(rootNode.childCount, 4); let node = rootNode.getChild(0); do_check_true(node.dateAdded > 0); @@ -711,6 +733,20 @@ function testSimpleFolderResult() { do_check_eq(node.title, "test folder"); do_check_true(node.dateAdded > 0); do_check_true(node.lastModified > 0); + node = rootNode.getChild(3); + do_check_eq(node.itemId, folderLongName); + do_check_eq(node.title, longName.substring(0, TITLE_LENGTH_MAX)); + do_check_true(node.dateAdded > 0); + do_check_true(node.lastModified > 0); + + // update with another long title + bs.setItemTitle(folderLongName, longName + " updated"); + do_check_eq(bookmarksObserver._itemChangedId, folderLongName); + do_check_eq(bookmarksObserver._itemChangedProperty, "title"); + do_check_eq(bookmarksObserver._itemChangedValue, longName.substring(0, TITLE_LENGTH_MAX)); + + node = rootNode.getChild(3); + do_check_eq(node.title, longName.substring(0, TITLE_LENGTH_MAX)); rootNode.containerOpen = false; } diff --git a/toolkit/components/places/tests/head_common.js b/toolkit/components/places/tests/head_common.js index 9d406e816b2..8ffa079977b 100644 --- a/toolkit/components/places/tests/head_common.js +++ b/toolkit/components/places/tests/head_common.js @@ -55,6 +55,8 @@ const TRANSITION_DOWNLOAD = Ci.nsINavHistoryService.TRANSITION_DOWNLOAD; // nsIFaviconService.idl, aboutCertError.xhtml and netError.xhtml. const FAVICON_ERRORPAGE_URL = "chrome://global/skin/icons/warning-16.png"; +const TITLE_LENGTH_MAX = 4096; + Cu.import("resource://gre/modules/XPCOMUtils.jsm"); XPCOMUtils.defineLazyGetter(this, "Services", function() {