diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index fc1fc3d1961e..e2796894369d 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -587,6 +587,11 @@ pref("browser.places.importBookmarksHTML", true); // if false, will add the "Smart Bookmarks" folder to the personal toolbar pref("browser.places.createdSmartBookmarks", false); +// If true, will migrate uri post-data annotations to +// bookmark post-data annotations (bug 398914) +// XXX to be removed after beta 2 (bug 391419) +pref("browser.places.migratePostDataAnnotations", true); + // Controls behavior of the "Add Exception" dialog launched from SSL error pages // 0 - don't pre-populate anything // 1 - pre-populate site URL, but don't fetch certificate diff --git a/browser/base/content/browser-places.js b/browser/base/content/browser-places.js index 2f090616b9f1..d0705bdb5ae7 100644 --- a/browser/base/content/browser-places.js +++ b/browser/base/content/browser-places.js @@ -822,3 +822,37 @@ var PlacesStarButton = { onItemVisited: function() { }, onItemMoved: function() { } }; + +/** + * Various migration tasks. + */ +function placesMigrationTasks() { + // bug 398914 - move all post-data annotations from URIs to bookmarks + // XXX - REMOVE ME FOR BETA 3 (bug 391419) + if (gPrefService.getBoolPref("browser.places.migratePostDataAnnotations")) { + const annosvc = PlacesUtils.annotations; + const bmsvc = PlacesUtils.bookmarks; + const oldPostDataAnno = "URIProperties/POSTData"; + var pages = annosvc.getPagesWithAnnotation(oldPostDataAnno, {}); + for (let i = 0; i < pages.length; i++) { + try { + let uri = pages[i]; + var postData = annosvc.getPageAnnotation(uri, oldPostDataAnno); + // We can't know which URI+keyword combo this postdata was for, but + // it's very likely that if this URI is bookmarked and has a keyword + // *and* the URI has postdata, then this bookmark was using the + // postdata. Propagate the annotation to all bookmarks for this URI + // just to be safe. + let bookmarks = bmsvc.getBookmarkIdsForURI(uri, {}); + for (let i = 0; i < bookmarks.length; i++) { + var keyword = bmsvc.getKeywordForBookmark(bookmarks[i]); + if (keyword) + annosvc.setItemAnnotation(bookmarks[i], POST_DATA_ANNO, postData, 0, annosvc.EXPIRE_NEVER); + } + // Remove the old annotation. + annosvc.removePageAnnotation(uri, oldPostDataAnno); + } catch(ex) {} + } + gPrefService.setBoolPref("browser.places.migratePostDataAnnotations", false); + } +} diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index e429819970b1..93e0b59ebf91 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -912,7 +912,10 @@ function delayedStartup() } UpdateUrlbarSearchSplitterState(); - + + try { + placesMigrationTasks(); + } catch(ex) {} initBookmarksToolbar(); PlacesStarButton.init(); @@ -1661,11 +1664,8 @@ function getShortcutOrURI(aURL, aPostDataRef) { if (engine) return engine.getSubmission(param, null).uri.spec; - try { - var shortcutURI = PlacesUtils.bookmarks.getURIForKeyword(keyword); - shortcutURL = shortcutURI.spec; - aPostDataRef.value = PlacesUtils.getPostDataForURI(shortcutURI); - } catch(ex) {} + [shortcutURL, aPostDataRef.value] = + PlacesUtils.getURLAndPostDataForKeyword(keyword); if (!shortcutURL) return aURL; diff --git a/browser/components/places/content/bookmarkProperties.js b/browser/components/places/content/bookmarkProperties.js index 147b4a32d150..8cf559f4be1a 100755 --- a/browser/components/places/content/bookmarkProperties.js +++ b/browser/components/places/content/bookmarkProperties.js @@ -929,16 +929,16 @@ var BookmarkPropertiesPanel = { PlacesUtils.ptm.editBookmarkMicrosummary(-1, microsummary)); } + if (this._postData) { + childTransactions.push( + PlacesUtils.ptm.editBookmarkPostData(-1, this._postData)); + } + var transactions = [PlacesUtils.ptm.createItem(uri, aContainer, aIndex, title, keyword, annotations, childTransactions)]; - if (this._postData) { - transactions.push( - PlacesUtils.ptm.editURIPostData(uri, this._postData)); - } - return PlacesUtils.ptm.aggregateTransactions(this._getDialogTitle(), transactions); }, diff --git a/browser/components/places/content/utils.js b/browser/components/places/content/utils.js index 2f31e2c826e0..fc9bc80cb4e5 100644 --- a/browser/components/places/content/utils.js +++ b/browser/components/places/content/utils.js @@ -50,7 +50,7 @@ Components.utils.import("resource://gre/modules/JSON.jsm"); const LOAD_IN_SIDEBAR_ANNO = "bookmarkProperties/loadInSidebar"; const DESCRIPTION_ANNO = "bookmarkProperties/description"; -const POST_DATA_ANNO = "URIProperties/POSTData"; +const POST_DATA_ANNO = "bookmarkProperties/POSTData"; const LMANNO_FEEDURI = "livemark/feedURI"; const LMANNO_SITEURI = "livemark/siteURI"; const ORGANIZER_QUERY_ANNO = "PlacesOrganizer/OrganizerQuery"; @@ -1446,33 +1446,58 @@ var PlacesUtils = { }, /** - * Set the POST data associated with a URI, if any. + * Set the POST data associated with a bookmark, if any. * Used by POST keywords. - * @param aURI + * @param aBookmarkId * @returns string of POST data */ - setPostDataForURI: function PU_setPostDataForURI(aURI, aPostData) { + setPostDataForBookmark: function PU_setPostDataForBookmark(aBookmarkId, aPostData) { const annos = this.annotations; if (aPostData) - annos.setPageAnnotation(aURI, POST_DATA_ANNO, aPostData, + annos.setItemAnnotation(aBookmarkId, POST_DATA_ANNO, aPostData, 0, Ci.nsIAnnotationService.EXPIRE_NEVER); - else if (annos.pageHasAnnotation(aURI, POST_DATA_ANNO)) - annos.removePageAnnotation(aURI, POST_DATA_ANNO); + else if (annos.itemHasAnnotation(aBookmarkId, POST_DATA_ANNO)) + annos.removeItemAnnotation(aBookmarkId, POST_DATA_ANNO); }, /** * Get the POST data associated with a bookmark, if any. - * @param aURI - * @returns string of POST data if set for aURI. null otherwise. + * @param aBookmarkId + * @returns string of POST data if set for aBookmarkId. null otherwise. */ - getPostDataForURI: function PU_getPostDataForURI(aURI) { + getPostDataForBookmark: function PU_getPostDataForBookmark(aBookmarkId) { const annos = this.annotations; - if (annos.pageHasAnnotation(aURI, POST_DATA_ANNO)) - return annos.getPageAnnotation(aURI, POST_DATA_ANNO); + if (annos.itemHasAnnotation(aBookmarkId, POST_DATA_ANNO)) + return annos.getItemAnnotation(aBookmarkId, POST_DATA_ANNO); return null; }, + /** + * Get the URI (and any associated POST data) for a given keyword. + * @param aKeyword string keyword + * @returns an array containing a string URL and a string of POST data + */ + getURLAndPostDataForKeyword: function PU_getURLAndPostDataForKeyword(aKeyword) { + var url = null, postdata = null; + try { + var uri = this.bookmarks.getURIForKeyword(aKeyword); + if (uri) { + url = uri.spec; + var bookmarks = this.bookmarks.getBookmarkIdsForURI(uri, {}); + for (let i = 0; i < bookmarks.length; i++) { + var bookmark = bookmarks[i]; + var kw = this.bookmarks.getKeywordForBookmark(bookmark); + if (kw == aKeyword) { + postdata = this.getPostDataForBookmark(bookmark); + break; + } + } + } + } catch(ex) {} + return [url, postdata]; + }, + /** * Retrieve the description of an item * @param aItemId diff --git a/browser/components/places/public/nsIPlacesTransactionsService.idl b/browser/components/places/public/nsIPlacesTransactionsService.idl index 19c67bc52cb7..251cff7725f9 100644 --- a/browser/components/places/public/nsIPlacesTransactionsService.idl +++ b/browser/components/places/public/nsIPlacesTransactionsService.idl @@ -52,7 +52,7 @@ interface nsITransaction; * the global scope of a js window. */ -[scriptable, uuid(939bccbd-ecb1-4742-9c38-a33af91ec872)] +[scriptable, uuid(89f61a91-c8f7-4abb-b880-895cb9852c35)] interface nsIPlacesTransactionsService : nsITransactionManager { /** @@ -237,20 +237,20 @@ interface nsIPlacesTransactionsService : nsITransactionManager * new keyword for the bookmark * @returns nsITransaction object */ - nsITransaction editBookmarkKeyword(in long long id, - in AString newKeyword); + nsITransaction editBookmarkKeyword(in long long aItemId, + in AString aNewKeyword); /** - * Transaction for editing the post data associated with a URI + * Transaction for editing the post data associated with a bookmark. * - * @param aURI - * uri to edit + * @param aItemId + * id of the bookmark to edit * @param aPostData * post data * @returns nsITransaction object */ - nsITransaction editURIPostData(in nsIURI aURI, - in AString aPostData); + nsITransaction editBookmarkPostData(in long long aItemId, + in AString aPostData); /** * Transaction for editing a live bookmark's site URI. diff --git a/browser/components/places/src/nsPlacesImportExportService.cpp b/browser/components/places/src/nsPlacesImportExportService.cpp index 1a4146bdab7b..11fbd5f48020 100644 --- a/browser/components/places/src/nsPlacesImportExportService.cpp +++ b/browser/components/places/src/nsPlacesImportExportService.cpp @@ -127,7 +127,7 @@ static NS_DEFINE_CID(kParserCID, NS_PARSER_CID); #define LOAD_IN_SIDEBAR_ANNO NS_LITERAL_CSTRING("bookmarkProperties/loadInSidebar") #define DESCRIPTION_ANNO NS_LITERAL_CSTRING("bookmarkProperties/description") -#define POST_DATA_ANNO NS_LITERAL_CSTRING("URIProperties/POSTData") +#define POST_DATA_ANNO NS_LITERAL_CSTRING("bookmarkProperties/POSTData") #define LAST_CHARSET_ANNO NS_LITERAL_CSTRING("URIProperties/characterSet") #define STATIC_TITLE_ANNO NS_LITERAL_CSTRING("bookmarks/staticTitle") @@ -950,7 +950,7 @@ BookmarkContentSink::HandleLinkBegin(const nsIParserNode& node) // post data if (!postData.IsEmpty()) { - mAnnotationService->SetPageAnnotationString(frame.mPreviousLink, POST_DATA_ANNO, + mAnnotationService->SetItemAnnotationString(frame.mPreviousId, POST_DATA_ANNO, postData, 0, nsIAnnotationService::EXPIRE_NEVER); } @@ -1855,12 +1855,12 @@ nsPlacesImportExportService::WriteItem(nsINavHistoryResultNode* aItem, NS_ENSURE_SUCCESS(rv, rv); PRBool hasPostData; - rv = mAnnotationService->PageHasAnnotation(pageURI, POST_DATA_ANNO, + rv = mAnnotationService->ItemHasAnnotation(itemId, POST_DATA_ANNO, &hasPostData); NS_ENSURE_SUCCESS(rv, rv); if (hasPostData) { nsAutoString postData; - rv = mAnnotationService->GetPageAnnotationString(pageURI, POST_DATA_ANNO, + rv = mAnnotationService->GetItemAnnotationString(itemId, POST_DATA_ANNO, postData); NS_ENSURE_SUCCESS(rv, rv); rv = aOutput->Write(kPostDataAttribute, sizeof(kPostDataAttribute)-1, &dummy); diff --git a/browser/components/places/src/nsPlacesTransactionsService.js b/browser/components/places/src/nsPlacesTransactionsService.js index ca103768ba79..fc005e763c2c 100644 --- a/browser/components/places/src/nsPlacesTransactionsService.js +++ b/browser/components/places/src/nsPlacesTransactionsService.js @@ -116,12 +116,12 @@ placesTransactionsService.prototype = { return new placesEditItemDescriptionTransactions(aItemId, aDescription); }, - editBookmarkKeyword: function placesEditBkmkKwd(id, newKeyword) { - return new placesEditBookmarkKeywordTransactions(id, newKeyword); + editBookmarkKeyword: function placesEditBkmkKwd(aItemId, newKeyword) { + return new placesEditBookmarkKeywordTransactions(aItemId, newKeyword); }, - editURIPostData: function placesEditURIPdata(aURI, aPostData) { - return new placesEditURIPostDataTransactions(aURI, aPostData); + editBookmarkPostData: function placesEditBookmarkPostdata(aItemId, aPostData) { + return new placesEditBookmarkPostDataTransactions(aItemId, aPostData); }, editLivemarkSiteURI: function placesEditLvmkSiteURI(folderId, uri) { @@ -206,7 +206,7 @@ placesBaseTransaction.prototype = { }, // nsITransaction - redoTransaction: function PIT_redoTransaction() { + redoTransaction: function PBT_redoTransaction() { throw Cr.NS_ERROR_NOT_IMPLEMENTED; }, @@ -640,23 +640,23 @@ placesEditBookmarkKeywordTransactions.prototype = { } }; -function placesEditURIPostDataTransactions(aURI, aPostData) { - this._uri = aURI; +function placesEditBookmarkPostDataTransactions(aItemId, aPostData) { + this.id = aItemId; this._newPostData = aPostData; this._oldPostData = null; this.redoTransaction = this.doTransaction; } -placesEditURIPostDataTransactions.prototype = { +placesEditBookmarkPostDataTransactions.prototype = { __proto__: placesBaseTransaction.prototype, doTransaction: function PEUPDT_doTransaction() { - this._oldPostData = PlacesUtils.getPostDataForURI(this._uri); - PlacesUtils.setPostDataForURI(this._uri, this._newPostData); + this._oldPostData = PlacesUtils.getPostDataForBookmark(this._id); + PlacesUtils.setPostDataForBookmark(this.id, this._newPostData); }, undoTransaction: function PEUPDT_undoTransaction() { - PlacesUtils.setPostDataForURI(this._uri, this._oldPostData); + PlacesUtils.setPostDataForBookmark(this.id, this._oldPostData); } }; diff --git a/browser/components/places/tests/unit/head_bookmarks.js b/browser/components/places/tests/unit/head_bookmarks.js index f9d2e3572648..df5fcd809064 100644 --- a/browser/components/places/tests/unit/head_bookmarks.js +++ b/browser/components/places/tests/unit/head_bookmarks.js @@ -38,9 +38,9 @@ * ***** END LICENSE BLOCK ***** */ const NS_APP_USER_PROFILE_50_DIR = "ProfD"; -const Ci = Components.interfaces; -const Cc = Components.classes; -const Cr = Components.results; +var Ci = Components.interfaces; +var Cc = Components.classes; +var Cr = Components.results; function LOG(aMsg) { aMsg = ("*** PLACES TESTS: " + aMsg); diff --git a/browser/components/places/tests/unit/test_398914.js b/browser/components/places/tests/unit/test_398914.js new file mode 100644 index 000000000000..d781b9297ea8 --- /dev/null +++ b/browser/components/places/tests/unit/test_398914.js @@ -0,0 +1,122 @@ +/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim:set ts=2 sw=2 sts=2 et: */ +/* ***** BEGIN LICENSE BLOCK ***** + * Version: MPL 1.1/GPL 2.0/LGPL 2.1 + * + * The contents of this file are subject to the Mozilla Public License Version + * 1.1 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * http://www.mozilla.org/MPL/ + * + * Software distributed under the License is distributed on an "AS IS" basis, + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License + * for the specific language governing rights and limitations under the + * License. + * + * The Original Code is mozilla.com code. + * + * The Initial Developer of the Original Code is Mozilla Corp. + * Portions created by the Initial Developer are Copyright (C) 2007 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * Dietrich Ayala + * + * Alternatively, the contents of this file may be used under the terms of + * either the GNU General Public License Version 2 or later (the "GPL"), or + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), + * in which case the provisions of the GPL or the LGPL are applicable instead + * of those above. If you wish to allow use of your version of this file only + * under the terms of either the GPL or the LGPL, and not to allow others to + * use your version of this file under the terms of the MPL, indicate your + * decision by deleting the provisions above and replace them with the notice + * and other provisions required by the GPL or the LGPL. If you do not delete + * the provisions above, a recipient may use your version of this file under + * the terms of any one of the MPL, the GPL or the LGPL. + * + * ***** END LICENSE BLOCK ***** */ + +version(170); + +var loader = Cc["@mozilla.org/moz/jssubscript-loader;1"]. + getService(Ci.mozIJSSubScriptLoader); +loader.loadSubScript("chrome://global/content/debug.js"); +loader.loadSubScript("chrome://browser/content/places/utils.js"); + +const bmsvc = PlacesUtils.bookmarks; +const testFolderId = PlacesUtils.bookmarksMenuFolderId; + +// main +function run_test() { + + var testURI = uri("http://foo.com"); + + /* + 1. Create a bookmark for a URI, with a keyword and post data. + 2. Create a bookmark for the same URI, with a different keyword and different post data. + 3. Confirm that our method for getting a URI+postdata retains bookmark affinity. + */ + var bm1 = bmsvc.insertBookmark(testFolderId, testURI, -1, "blah"); + bmsvc.setKeywordForBookmark(bm1, "foo"); + PlacesUtils.setPostDataForBookmark(bm1, "pdata1"); + var bm2 = bmsvc.insertBookmark(testFolderId, testURI, -1, "blah"); + bmsvc.setKeywordForBookmark(bm2, "bar"); + PlacesUtils.setPostDataForBookmark(bm2, "pdata2"); + + // check kw, pd for bookmark 1 + var url, postdata; + [url, postdata] = PlacesUtils.getURLAndPostDataForKeyword("foo"); + do_check_eq(testURI.spec, url); + do_check_eq(postdata, "pdata1"); + + // check kw, pd for bookmark 2 + [url, postdata] = PlacesUtils.getURLAndPostDataForKeyword("bar"); + do_check_eq(testURI.spec, url); + do_check_eq(postdata, "pdata2"); + + // cleanup + bmsvc.removeItem(bm1); + bmsvc.removeItem(bm2); + + /* + 1. Create two bookmarks with the same URI and keyword. + 2. Confirm that the most recently created one is returned for that keyword. + */ + var bm1 = bmsvc.insertBookmark(testFolderId, testURI, -1, "blah"); + bmsvc.setKeywordForBookmark(bm1, "foo"); + PlacesUtils.setPostDataForBookmark(bm1, "pdata1"); + var bm2 = bmsvc.insertBookmark(testFolderId, testURI, -1, "blah"); + bmsvc.setKeywordForBookmark(bm2, "foo"); + PlacesUtils.setPostDataForBookmark(bm2, "pdata2"); + + [url, postdata] = PlacesUtils.getURLAndPostDataForKeyword("foo"); + do_check_eq(testURI.spec, url); + do_check_eq(postdata, "pdata2"); + + // cleanup + bmsvc.removeItem(bm1); + bmsvc.removeItem(bm2); + + /* + 1. Create two bookmarks with the same URI and keyword. + 2. Modify the first-created bookmark. + 3. Confirm that the most recently modified one is returned for that keyword. + */ + var bm1 = bmsvc.insertBookmark(testFolderId, testURI, -1, "blah"); + bmsvc.setKeywordForBookmark(bm1, "foo"); + PlacesUtils.setPostDataForBookmark(bm1, "pdata1"); + var bm2 = bmsvc.insertBookmark(testFolderId, testURI, -1, "blah"); + bmsvc.setKeywordForBookmark(bm2, "foo"); + PlacesUtils.setPostDataForBookmark(bm2, "pdata2"); + + // modify the older bookmark + bmsvc.setItemTitle(bm1, "change"); + + [url, postdata] = PlacesUtils.getURLAndPostDataForKeyword("foo"); + do_check_eq(testURI.spec, url); + do_check_eq(postdata, "pdata1"); + + // cleanup + bmsvc.removeItem(bm1); + bmsvc.removeItem(bm2); +} diff --git a/browser/components/places/tests/unit/test_bookmarks_html.js b/browser/components/places/tests/unit/test_bookmarks_html.js index de3c9da70232..39b0b3854733 100644 --- a/browser/components/places/tests/unit/test_bookmarks_html.js +++ b/browser/components/places/tests/unit/test_bookmarks_html.js @@ -81,7 +81,7 @@ try { const DESCRIPTION_ANNO = "bookmarkProperties/description"; const LOAD_IN_SIDEBAR_ANNO = "bookmarkProperties/loadInSidebar"; -const POST_DATA_ANNO = "URIProperties/POSTData"; +const POST_DATA_ANNO = "bookmarkProperties/POSTData"; const LAST_CHARSET_ANNO = "URIProperties/characterSet"; // main @@ -241,11 +241,12 @@ function testCanonicalBookmarks(aFolder) { do_check_eq(testBookmark1.lastModified/1000000, 1177375423); // post data - var pageURI = iosvc.newURI(testBookmark1.uri, "", null); - do_check_true(annosvc.pageHasAnnotation(pageURI, POST_DATA_ANNO)); + do_check_true(annosvc.itemHasAnnotation(testBookmark1.itemId, POST_DATA_ANNO)); do_check_eq("hidden1%3Dbar&text1%3D%25s", - annosvc.getPageAnnotation(pageURI, POST_DATA_ANNO)); + annosvc.getItemAnnotation(testBookmark1.itemId, POST_DATA_ANNO)); + // last charset + var pageURI = iosvc.newURI(testBookmark1.uri, "", null); do_check_true(annosvc.pageHasAnnotation(pageURI, LAST_CHARSET_ANNO)); do_check_eq("ISO-8859-1", annosvc.getPageAnnotation(pageURI, LAST_CHARSET_ANNO)); diff --git a/browser/components/places/tests/unit/test_placesTxn.js b/browser/components/places/tests/unit/test_placesTxn.js index 36d1aebc4e04..653ad5796f65 100644 --- a/browser/components/places/tests/unit/test_placesTxn.js +++ b/browser/components/places/tests/unit/test_placesTxn.js @@ -381,6 +381,17 @@ function run_test() { do_check_eq(observer._itemChangedId, bId); do_check_true(!mss.hasMicrosummary(bId)); - // Testing edit Post Data... - // mmm.. cant figure out a good way to test this. + // Testing edit Post Data + const POST_DATA_ANNO = "bookmarkProperties/POSTData"; + var postData = "foo"; + var postDataURI = uri("http://foo.com"); + ptSvc.doTransaction( + ptSvc.createItem(postDataURI, root, -1, "postdata test", null, null, null)); + var postDataId = (bmsvc.getBookmarkIdsForURI(postDataURI,{}))[0]; + var postDataTxn = ptSvc.editBookmarkPostData(postDataId, postData); + postDataTxn.doTransaction(); + do_check_true(annotationService.itemHasAnnotation(postDataId, POST_DATA_ANNO)) + do_check_eq(annotationService.getItemAnnotation(postDataId, POST_DATA_ANNO), postData); + postDataTxn.undoTransaction(); + do_check_false(annotationService.itemHasAnnotation(postDataId, POST_DATA_ANNO)) } diff --git a/toolkit/components/places/src/nsNavBookmarks.cpp b/toolkit/components/places/src/nsNavBookmarks.cpp index 8a46b888968e..955a4c243b85 100644 --- a/toolkit/components/places/src/nsNavBookmarks.cpp +++ b/toolkit/components/places/src/nsNavBookmarks.cpp @@ -139,6 +139,8 @@ nsNavBookmarks::Init() NS_ENSURE_SUCCESS(rv, rv); // mDBFindURIBookmarks + // NOTE: Do not modify the ORDER BY segment of the query, as certain + // features depend on it. See bug 398914 for an example. rv = dbConn->CreateStatement(NS_LITERAL_CSTRING( "SELECT a.id " "FROM moz_bookmarks a, moz_places h "