# HG changeset patch # User Frank-Rainer Grahl # Parent b014a832b799ee2396c19e82eea29aec718d8c97 Bug 1439220 - Align smart bookmark initialisation with browser. Port Bug 1437040 [Remove synchronous Bookmarks::GetItemIndex" to SeaMonkey]. Port Bug 1094888 [Smart bookmarks creation in nsBrowserGlue should use the new Bookmarks.jsm API]. diff --git a/suite/common/public/nsISuiteGlue.idl b/suite/common/public/nsISuiteGlue.idl --- a/suite/common/public/nsISuiteGlue.idl +++ b/suite/common/public/nsISuiteGlue.idl @@ -18,30 +18,25 @@ interface nsIDOMWindow; * Dued to its nature and origin, this interface won't probably be the most * elegant or stable in the mozilla codebase, but its aim is rather pragmatic: * 1) reducing the performance overhead which affects browser window load; * 2) allow global hooks (e.g. startup and shutdown observers) which survive * suite windows to accomplish suite-related activities, such as shutdown * sanitization (see bug #284086) */ -[scriptable, uuid(2559bb36-2eef-4190-9df1-8afcfbd35bd3)] +[scriptable, uuid(b3a787fd-4c05-4518-98e3-20bc10a0f586)] interface nsISuiteGlue : nsISupports { /** * Opens the Download Manager. */ void showDownloadManager(); /** * Deletes privacy sensitive data according to user preferences * * @param aParentWindow an optionally null window which is the parent of the * sanitization dialog (if it has to be shown per user preferences) * */ void sanitize(in nsIDOMWindow aParentWindow); - - /** - * Add Smart Bookmarks special queries to bookmarks menu and toolbar folder. - */ - void ensurePlacesDefaultQueriesInitialized(); }; diff --git a/suite/common/src/nsSuiteGlue.js b/suite/common/src/nsSuiteGlue.js --- a/suite/common/src/nsSuiteGlue.js +++ b/suite/common/src/nsSuiteGlue.js @@ -1028,25 +1028,28 @@ SuiteGlue.prototype = { } else { // ...otherwise we will restore defaults. restoreDefaultBookmarks = true; } } } - // If bookmarks are not imported, then initialize smart bookmarks. This + // If bookmarks are not imported, then initialize smart bookmarks. This // happens during a common startup. - // Otherwise, if any kind of import runs, smart bookmarks creation should be - // delayed till the import operations has finished. Not doing so would + // Otherwise, if any kind of import runs, smart bookmarks creation should + // be delayed till the import operations has finished. Not doing so would // cause them to be overwritten by the newly imported bookmarks. if (!importBookmarks) { - this.ensurePlacesDefaultQueriesInitialized(); - } - else { + try { + await this.ensurePlacesDefaultQueriesInitialized(); + } catch (e) { + Cu.reportError(e); + } + } else { // An import operation is about to run. // Don't try to recreate smart bookmarks if autoExportHTML is true or // smart bookmarks are disabled. var autoExportHTML = false; try { autoExportHTML = Services.prefs.getBoolPref("browser.bookmarks.autoExportHTML"); } catch(ex) {} var smartBookmarksVersion = 0; @@ -1073,17 +1076,17 @@ SuiteGlue.prototype = { try { BookmarkHTMLUtils.importFromURL(bookmarksURI.spec, true).then(null, function onFailure() { Cu.reportError("Bookmarks.html file could be corrupt."); } ).then( // Ensure that smart bookmarks are created once the operation is // complete. - this.ensurePlacesDefaultQueriesInitialized.bind(this) + await this.ensurePlacesDefaultQueriesInitialized.bind(this) ); } catch(ex) { Cu.reportError("bookmarks.html file could be corrupt. " + ex); } } else { Cu.reportError("Unable to find bookmarks.html file."); @@ -1378,159 +1381,149 @@ SuiteGlue.prototype = { sanitize: function(aParentWindow) { // call the Sanitizer object's sanitize, which might return errors // but do not forward them anywhere, as we are defined as void here Sanitizer.sanitize(aParentWindow); }, - ensurePlacesDefaultQueriesInitialized: - function BG_ensurePlacesDefaultQueriesInitialized() { - // This is actual version of the smart bookmarks, must be increased every - // time smart bookmarks change. + async ensurePlacesDefaultQueriesInitialized() { + // This is the current smart bookmarks version, it must be increased every + // time they change. // When adding a new smart bookmark below, its newInVersion property must - // be set to the version it has been added in, we will compare its value + // be set to the version it has been added in. We will compare its value // to users' smartBookmarksVersion and add new smart bookmarks without // recreating old deleted ones. - const SMART_BOOKMARKS_VERSION = 4; + const SMART_BOOKMARKS_VERSION = 7; const SMART_BOOKMARKS_ANNO = "Places/SmartBookmark"; const SMART_BOOKMARKS_PREF = "browser.places.smartBookmarksVersion"; // TODO bug 399268: should this be a pref? const MAX_RESULTS = 10; // Get current smart bookmarks version. If not set, create them. - let smartBookmarksCurrentVersion = 0; - try { - smartBookmarksCurrentVersion = Services.prefs.getIntPref(SMART_BOOKMARKS_PREF); - } catch(ex) {} + let smartBookmarksCurrentVersion = Services.prefs.getIntPref(SMART_BOOKMARKS_PREF, 0); - // If version is current or smart bookmarks are disabled, just bail out. + // If version is current, or smart bookmarks are disabled, bail out. if (smartBookmarksCurrentVersion == -1 || smartBookmarksCurrentVersion >= SMART_BOOKMARKS_VERSION) { return; } - let batch = { - runBatched: function BG_EPDQI_runBatched() { - let menuIndex = 0; - let toolbarIndex = 0; - let bundle = Services.strings.createBundle("chrome://communicator/locale/places/places.properties"); + try { + let menuIndex = 0; + let toolbarIndex = 0; + let bundle = Services.strings.createBundle("chrome://communicator/locale/places/places.properties"); + let queryOptions = Ci.nsINavHistoryQueryOptions; - let smartBookmarks = { - MostVisited: { - title: bundle.GetStringFromName("mostVisitedTitle"), - uri: NetUtil.newURI("place:sort=" + - Ci.nsINavHistoryQueryOptions.SORT_BY_VISITCOUNT_DESCENDING + - "&maxResults=" + MAX_RESULTS), - parent: PlacesUtils.toolbarFolderId, - position: toolbarIndex++, - newInVersion: 1 - }, - RecentlyBookmarked: { - title: bundle.GetStringFromName("recentlyBookmarkedTitle"), - uri: NetUtil.newURI("place:folder=BOOKMARKS_MENU" + - "&folder=UNFILED_BOOKMARKS" + - "&folder=TOOLBAR" + - "&queryType=" + - Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS + - "&sort=" + - Ci.nsINavHistoryQueryOptions.SORT_BY_DATEADDED_DESCENDING + - "&maxResults=" + MAX_RESULTS + - "&excludeQueries=1"), - parent: PlacesUtils.bookmarksMenuFolderId, - position: menuIndex++, - newInVersion: 1 - }, - RecentTags: { - title: bundle.GetStringFromName("recentTagsTitle"), - uri: NetUtil.newURI("place:"+ - "type=" + - Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY + - "&sort=" + - Ci.nsINavHistoryQueryOptions.SORT_BY_LASTMODIFIED_DESCENDING + - "&maxResults=" + MAX_RESULTS), - parent: PlacesUtils.bookmarksMenuFolderId, - position: menuIndex++, - newInVersion: 1 - } - }; + let smartBookmarks = { + MostVisited: { + title: bundle.GetStringFromName("mostVisitedTitle"), + url: "place:sort=" + queryOptions.SORT_BY_VISITCOUNT_DESCENDING + + "&maxResults=" + MAX_RESULTS, + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + newInVersion: 1 + }, + RecentlyBookmarked: { + title: bundle.GetStringFromName("recentlyBookmarkedTitle"), + url: "place:folder=BOOKMARKS_MENU" + "&folder=UNFILED_BOOKMARKS" + + "&folder=TOOLBAR" + + "&queryType=" + queryOptions.QUERY_TYPE_BOOKMARKS + + "&sort=" + queryOptions.SORT_BY_DATEADDED_DESCENDING + + "&maxResults=" + MAX_RESULTS + + "&excludeQueries=1", + parentGuid: PlacesUtils.bookmarks.menuGuid, + newInVersion: 1 + }, + RecentTags: { + title: bundle.GetStringFromName("recentTagsTitle"), + url: "place:type=" + queryOptions.RESULTS_AS_TAG_QUERY + + "&sort=" + queryOptions.SORT_BY_LASTMODIFIED_DESCENDING + + "&maxResults=" + MAX_RESULTS, + parentGuid: PlacesUtils.bookmarks.menuGuid, + newInVersion: 1 + }, + }; - // Set current itemId, parent and position if Smart Bookmark exists, - // we will use these informations to create the new version at the same - // position. - let smartBookmarkItemIds = PlacesUtils.annotations.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO); - smartBookmarkItemIds.forEach(function (itemId) { - let queryId = PlacesUtils.annotations.getItemAnnotation(itemId, SMART_BOOKMARKS_ANNO); - if (queryId in smartBookmarks) { - let smartBookmark = smartBookmarks[queryId]; - smartBookmark.itemId = itemId; - smartBookmark.parent = PlacesUtils.bookmarks.getFolderIdForItem(itemId); - smartBookmark.position = PlacesUtils.bookmarks.getItemIndex(itemId); - } else { - // We don't remove old Smart Bookmarks because user could still - // find them useful, or could have personalized them. - // Instead we remove the Smart Bookmark annotation. - PlacesUtils.annotations.removeItemAnnotation(itemId, SMART_BOOKMARKS_ANNO); - } - }); + // Set current guid, parentGuid and index of existing Smart Bookmarks. + // We will use those to create a new version of the bookmark at the same + // position. + let smartBookmarkItemIds = PlacesUtils.annotations.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO); + for (let itemId of smartBookmarkItemIds) { + let queryId = PlacesUtils.annotations.getItemAnnotation(itemId, SMART_BOOKMARKS_ANNO); + if (queryId in smartBookmarks) { + // Known smart bookmark. + let smartBookmark = smartBookmarks[queryId]; + smartBookmark.guid = await PlacesUtils.promiseItemGuid(itemId); - for (let queryId in smartBookmarks) { - let smartBookmark = smartBookmarks[queryId]; - - // We update or create only changed or new smart bookmarks. - // Also we respect user choices, so we won't try to create a smart - // bookmark if it has been removed. - if (smartBookmarksCurrentVersion > 0 && - smartBookmark.newInVersion <= smartBookmarksCurrentVersion && - !smartBookmark.itemId) + if (!smartBookmark.url) { + await PlacesUtils.bookmarks.remove(smartBookmark.guid); continue; - - // Remove old version of the smart bookmark if it exists, since it - // will be replaced in place. - if (smartBookmark.itemId) { - PlacesUtils.bookmarks.removeItem(smartBookmark.itemId); } - // Create the new smart bookmark and store its updated itemId. - smartBookmark.itemId = - PlacesUtils.bookmarks.insertBookmark(smartBookmark.parent, - smartBookmark.uri, - smartBookmark.position, - smartBookmark.title); - PlacesUtils.annotations.setItemAnnotation(smartBookmark.itemId, - SMART_BOOKMARKS_ANNO, - queryId, 0, - PlacesUtils.annotations.EXPIRE_NEVER); + let bm = await PlacesUtils.bookmarks.fetch(smartBookmark.guid); + smartBookmark.parentGuid = bm.parentGuid; + smartBookmark.index = bm.index; + } else { + // We don't remove old Smart Bookmarks because user could still + // find them useful, or could have personalized them. + // Instead we remove the Smart Bookmark annotation. + PlacesUtils.annotations.removeItemAnnotation(itemId, SMART_BOOKMARKS_ANNO); + } + } + + for (let queryId of Object.keys(smartBookmarks)) { + let smartBookmark = smartBookmarks[queryId]; + + // We update or create only changed or new smart bookmarks. + // Also we respect user choices, so we won't try to create a smart + // bookmark if it has been removed. + if (smartBookmarksCurrentVersion > 0 && + smartBookmark.newInVersion <= smartBookmarksCurrentVersion && + !smartBookmark.guid || !smartBookmark.url) + continue; + + // Remove old version of the smart bookmark if it exists, since it + // will be replaced in place. + if (smartBookmark.guid) { + await PlacesUtils.bookmarks.remove(smartBookmark.guid); } - // If we are creating all Smart Bookmarks from ground up, add a - // separator below them in the bookmarks menu. - if (smartBookmarksCurrentVersion == 0 && - smartBookmarkItemIds.length == 0) { - let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId, - menuIndex); - // Don't add a separator if the menu was empty or there is one already. - if (id != -1 && - PlacesUtils.bookmarks.getItemType(id) != PlacesUtils.bookmarks.TYPE_SEPARATOR) { - PlacesUtils.bookmarks.insertSeparator(PlacesUtils.bookmarksMenuFolderId, - menuIndex); - } + // Create the new smart bookmark and store its updated guid. + if (!("index" in smartBookmark)) { + if (smartBookmark.parentGuid == PlacesUtils.bookmarks.toolbarGuid) + smartBookmark.index = toolbarIndex++; + else if (smartBookmark.parentGuid == PlacesUtils.bookmarks.menuGuid) + smartBookmark.index = menuIndex++; + } + smartBookmark = await PlacesUtils.bookmarks.insert(smartBookmark); + let itemId = await PlacesUtils.promiseItemId(smartBookmark.guid); + PlacesUtils.annotations.setItemAnnotation(itemId, + SMART_BOOKMARKS_ANNO, + queryId, 0, + PlacesUtils.annotations.EXPIRE_NEVER); + } + + // If we are creating all Smart Bookmarks from ground up, add a + // separator below them in the bookmarks menu. + if (smartBookmarksCurrentVersion == 0 && + smartBookmarkItemIds.length == 0) { + let bm = await PlacesUtils.bookmarks.fetch({ parentGuid: PlacesUtils.bookmarks.menuGuid, + index: menuIndex }); + // Don't add a separator if the menu was empty or there is one already. + if (bm && bm.type != PlacesUtils.bookmarks.TYPE_SEPARATOR) { + await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_SEPARATOR, + parentGuid: PlacesUtils.bookmarks.menuGuid, + index: menuIndex }); } } - }; - - try { - PlacesUtils.bookmarks.runInBatchMode(batch, null); - } - catch(ex) { + } catch (ex) { Cu.reportError(ex); - } - finally { + } finally { Services.prefs.setIntPref(SMART_BOOKMARKS_PREF, SMART_BOOKMARKS_VERSION); Services.prefs.savePrefFile(null); } }, /** * Called as an observer when Sync's "display URI" notification is fired. */