import changes from `dev' branch of rmottola/Arctic-Fox:

- Bug 951651 - Make bookmarkProperties, Star UI and Library info pane work with PlacesTransactions. r=mak (6d0127aa5)
- Bug 547623 - Add a button to about:support to enter safe mode. r=adw (1a0481412)
- Bug 728813 - Switch Help menu item to allow leaving Safe Mode when it's enabled. r=MattN (cfeae5bf8)
- Bug 1125115 - Write a new keywords pseudo-API in PlacesUtils. r=ttaubert (afc6f00c9)

browser/base/content/baseMenuOverlay.xul

@@ -65,7 +65,9 @@
         <menuitem id="helpSafeMode"
                   accesskey="&helpSafeMode.accesskey;"
                   label="&helpSafeMode.label;"
-                  oncommand="restart(true);"/>
+                  stopaccesskey="&helpSafeMode.stop.accesskey;"
+                  stoplabel="&helpSafeMode.stop.label;"
+                  oncommand="safeModeRestart();"/>
         <menuseparator id="aboutSeparator"/>
         <menuitem id="aboutName"
                   accesskey="&aboutProduct.accesskey;"

browser/base/content/browser-places.js

@@ -56,7 +56,7 @@ var StarUI = {
   },
 
   // nsIDOMEventListener
-  handleEvent: function SU_handleEvent(aEvent) {
+  handleEvent(aEvent) {
     switch (aEvent.type) {
       case "popuphidden":
         if (aEvent.originalTarget == this.panel) {
@@ -65,26 +65,32 @@ var StarUI = {
 
           this._restoreCommandsState();
           this._itemId = -1;
-          if (this._batching) {
-            PlacesUtils.transactionManager.endBatch(false);
-            this._batching = false;
-          }
+          if (this._batching)
+            this.endBatch();
 
           switch (this._actionOnHide) {
             case "cancel": {
-              PlacesUtils.transactionManager.undoTransaction();
+              if (!PlacesUIUtils.useAsyncTransactions) {
+                PlacesUtils.transactionManager.undoTransaction();
+                break;
+              }
+              PlacesTransactions.undo().catch(Cu.reportError);
               break;
             }
             case "remove": {
               // Remove all bookmarks for the bookmark's url, this also removes
               // the tags for the url.
-              PlacesUtils.transactionManager.beginBatch(null);
-              let itemIds = PlacesUtils.getBookmarksForURI(this._uriForRemoval);
-              for (let i = 0; i < itemIds.length; i++) {
-                let txn = new PlacesRemoveItemTransaction(itemIds[i]);
-                PlacesUtils.transactionManager.doTransaction(txn);
+              if (!PlacesUIUtils.useAsyncTransactions) {
+                let itemIds = PlacesUtils.getBookmarksForURI(this._uriForRemoval);
+                for (let itemId of itemIds) {
+                  let txn = new PlacesRemoveItemTransaction(itemId);
+                  PlacesUtils.transactionManager.doTransaction(txn);
+                }
+                break;
               }
-              PlacesUtils.transactionManager.endBatch(false);
+
+              PlacesTransactions.RemoveBookmarksForUrls(this._uriForRemoval)
+                                .transact().catch(Cu.reportError);
               break;
             }
           }
@@ -118,15 +124,30 @@ var StarUI = {
 
   _overlayLoaded: false,
   _overlayLoading: false,
-  showEditBookmarkPopup:
-  function SU_showEditBookmarkPopup(aItemId, aAnchorElement, aPosition) {
+  showEditBookmarkPopup: Task.async(function* (aNode, aAnchorElement, aPosition) {
+    // TODO: Deprecate this once async transactions are enabled and the legacy
+    // transactions code is gone (bug 1131491) - we don't want addons to to use
+    // the  completeNodeLikeObjectForItemId, so it's better if they keep passing
+    // the item-id for now).
+    if (typeof(aNode) == "number") {
+      let itemId = aNode;
+      if (PlacesUIUtils.useAsyncTransactions) {
+        let guid = yield PlacesUtils.promiseItemGuid(itemId);
+        aNode = yield PlacesUIUtils.promiseNodeLike(guid);
+      }
+      else {
+        aNode = { itemId };
+        yield PlacesUIUtils.completeNodeLikeObjectForItemId(aNode);
+      }
+    }
+
     // Performance: load the overlay the first time the panel is opened
     // (see bug 392443).
     if (this._overlayLoading)
       return;
 
     if (this._overlayLoaded) {
-      this._doShowEditBookmarkPanel(aItemId, aAnchorElement, aPosition);
+      this._doShowEditBookmarkPanel(aNode, aAnchorElement, aPosition);
       return;
     }
 
@@ -146,13 +167,12 @@ var StarUI = {
 
         this._overlayLoading = false;
         this._overlayLoaded = true;
-        this._doShowEditBookmarkPanel(aItemId, aAnchorElement, aPosition);
+        this._doShowEditBookmarkPanel(aNode, aAnchorElement, aPosition);
       }).bind(this)
     );
-  },
+  }),
 
-  _doShowEditBookmarkPanel:
-  function SU__doShowEditBookmarkPanel(aItemId, aAnchorElement, aPosition) {
+  _doShowEditBookmarkPanel: Task.async(function* (aNode, aAnchorElement, aPosition) {
     if (this.panel.state != "closed")
       return;
 
@@ -178,23 +198,23 @@ var StarUI = {
 
     // The label of the remove button differs if the URI is bookmarked
     // multiple times.
-    var bookmarks = PlacesUtils.getBookmarksForURI(gBrowser.currentURI);
-    var forms = gNavigatorBundle.getString("editBookmark.removeBookmarks.label");
-    var label = PluralForm.get(bookmarks.length, forms).replace("#1", bookmarks.length);
+    let bookmarks = PlacesUtils.getBookmarksForURI(gBrowser.currentURI);
+    let forms = gNavigatorBundle.getString("editBookmark.removeBookmarks.label");
+    let label = PluralForm.get(bookmarks.length, forms).replace("#1", bookmarks.length);
     this._element("editBookmarkPanelRemoveButton").label = label;
 
     // unset the unstarred state, if set
     this._element("editBookmarkPanelStarIcon").removeAttribute("unstarred");
 
-    this._itemId = aItemId !== undefined ? aItemId : this._itemId;
+    this._itemId = aNode.itemId;
     this.beginBatch();
 
     this.panel.openPopup(aAnchorElement, aPosition);
 
-    gEditItemOverlay.initPanel(this._itemId,
-                               { hiddenRows: ["description", "location",
+    gEditItemOverlay.initPanel({ node: aNode
+                               , hiddenRows: ["description", "location",
                                               "loadInSidebar", "keyword"] });
-  },
+  }),
 
   panelShown:
   function SU_panelShown(aEvent) {
@@ -231,13 +251,46 @@ var StarUI = {
     this.panel.hidePopup();
   },
 
-  beginBatch: function SU_beginBatch() {
-    if (!this._batching) {
-      PlacesUtils.transactionManager.beginBatch(null);
-      this._batching = true;
+  // Matching the way it is used in the Library, editBookmarkOverlay implements
+  // an instant-apply UI, having no batched-Undo/Redo support.
+  // However, in this context (the Star UI) we have a Cancel button whose
+  // expected behavior is to undo all the operations done in the panel.
+  // Sometime in the future this needs to be reimplemented using a
+  // non-instant apply code path, but for the time being, we patch-around
+  // editBookmarkOverlay so that all of the actions done in the panel
+  // are treated by PlacesTransactions as a single batch.  To do so,
+  // we start a PlacesTransactions batch when the star UI panel is shown, and
+  // we keep the batch ongoing until the panel is hidden.
+  _batchBlockingDeferred: null,
+  beginBatch() {
+    if (this._batching)
+      return;
+    if (PlacesUIUtils.useAsyncTransactions) {
+      this._batchBlockingDeferred = PromiseUtils.defer();
+      PlacesTransactions.batch(function* () {
+        yield this._batchBlockingDeferred.promise;
+      }.bind(this));
     }
+    else {
+      PlacesUtils.transactionManager.beginBatch(null);
+    }
+    this._batching = true;
+  },
+
+  endBatch() {
+    if (!this._batching)
+      return;
+
+    if (PlacesUIUtils.useAsyncTransactions) {
+      this._batchBlockingDeferred.resolve();
+      this._batchBlockingDeferred = null;
+    }
+    else {
+      PlacesUtils.transactionManager.endBatch(false);
+    }
+    this._batching = false;
   }
-}
+};
 
 ////////////////////////////////////////////////////////////////////////////////
 //// PlacesCommandHook
@@ -253,8 +306,11 @@ var PlacesCommandHook = {
    *        aBrowser isn't bookmarked yet, defaults to the unfiled root.
    * @param [optional] aShowEditUI
    *        whether or not to show the edit-bookmark UI for the bookmark item
-   */  
-  bookmarkPage: function PCH_bookmarkPage(aBrowser, aParent, aShowEditUI) {
+   */
+  bookmarkPage: Task.async(function* (aBrowser, aParent, aShowEditUI) {
+    if (PlacesUIUtils.useAsyncTransactions)
+      return (yield this._bookmarkPagePT(aBrowser, aParent, aShowEditUI));
+
     var uri = aBrowser.currentURI;
     var itemId = PlacesUtils.getMostRecentBookmarkForURI(uri);
     if (itemId == -1) {
@@ -287,7 +343,7 @@ var PlacesCommandHook = {
         StarUI.beginBatch();
       }
 
-      var parent = aParent != undefined ?
+      var parent = aParent !== undefined ?
                    aParent : PlacesUtils.unfiledBookmarksFolderId;
       var descAnno = { name: PlacesUIUtils.DESCRIPTION_ANNO, value: description };
       var txn = new PlacesCreateBookmarkTransaction(uri, parent, 
@@ -295,7 +351,7 @@ var PlacesCommandHook = {
                                                     title, null, [descAnno]);
       PlacesUtils.transactionManager.doTransaction(txn);
       itemId = txn.item.id;
-      // Set the character-set
+      // Set the character-set.
       if (charset && !PrivateBrowsingUtils.isBrowserPrivate(aBrowser))
         PlacesUtils.setCharsetForURI(uri, charset);
     }
@@ -325,7 +381,82 @@ var PlacesCommandHook = {
     } else {
       StarUI.showEditBookmarkPopup(itemId, aBrowser, "overlap");
     }
-  },
+  }),
+
+  // TODO: Replace bookmarkPage code with this function once legacy
+  // transactions are removed.
+  _bookmarkPagePT: Task.async(function* (aBrowser, aParentId, aShowEditUI) {
+    let url = new URL(aBrowser.currentURI.spec);
+    let info = yield PlacesUtils.bookmarks.fetch({ url });
+    if (!info) {
+      let parentGuid = aParentId !== undefined ?
+                         yield PlacesUtils.promiseItemGuid(aParentId) :
+                         PlacesUtils.bookmarks.unfiledGuid;
+      info = { url, parentGuid };
+      // Bug 1148838 - Make this code work for full page plugins.
+      let description = null;
+      let charset = null;
+      try {
+        let isErrorPage = /^about:(neterror|certerror|blocked)/
+                          .test(aBrowser.contentDocumentAsCPOW.documentURI);
+        info.title = isErrorPage ?
+          (yield PlacesUtils.promisePlaceInfo(aBrowser.currentURI)).title :
+          aBrowser.contentTitle;
+        info.title = info.title || url.href;
+        description = PlacesUIUtils.getDescriptionFromDocument(aBrowser.contentDocumentAsCPOW);
+        charset = aBrowser.characterSet;
+      }
+      catch (e) {
+      	Components.utils.reportError(e);
+      }
+
+      if (aShowEditUI) {
+        // If we bookmark the page here (i.e. page was not "starred" already)
+        // but open right into the "edit" state, start batching here, so
+        // "Cancel" in that state removes the bookmark.
+        StarUI.beginBatch();
+      }
+
+      if (description) {
+        info.annotations = [{ name: PlacesUIUtils.DESCRIPTION_ANNO
+                            , value: description }];
+      }
+
+      info.guid = yield PlacesTransactions.NewBookmark(info).transact();
+
+      // Set the character-set
+      if (charset && !PrivateBrowsingUtils.isBrowserPrivate(aBrowser))
+      	 PlacesUtils.setCharsetForURI(makeURI(url.href), charset);
+    }
+
+    // Revert the contents of the location bar
+    if (gURLBar)
+      gURLBar.handleRevert();
+
+    // If it was not requested to open directly in "edit" mode, we are done.
+    if (!aShowEditUI)
+      return;
+
+    let node = yield PlacesUIUtils.promiseNodeLikeFromFetchInfo(info);
+
+    // Try to dock the panel to:
+    // 1. the bookmarks menu button
+    // 2. the page-proxy-favicon
+    // 3. the content area
+    if (BookmarkingUI.anchor) {
+      StarUI.showEditBookmarkPopup(node, BookmarkingUI.anchor,
+                                   "bottomcenter topright");
+      return;
+    }
+
+    let pageProxyFavicon = document.getElementById("page-proxy-favicon");
+    if (isElementVisible(pageProxyFavicon)) {
+      StarUI.showEditBookmarkPopup(node, pageProxyFavicon,
+                                   "bottomcenter topright");
+    } else {
+      StarUI.showEditBookmarkPopup(node, aBrowser, "overlap");
+    }
+  }),
 
   /**
    * Adds a bookmark to the page loaded in the current tab. 
@@ -344,10 +475,27 @@ var PlacesCommandHook = {
    * @param aTitle
    *        The link text
    */
-  bookmarkLink: function PCH_bookmarkLink(aParent, aURL, aTitle) {
-    var linkURI = makeURI(aURL);
-    var itemId = PlacesUtils.getMostRecentBookmarkForURI(linkURI);
-    if (itemId == -1) {
+  bookmarkLink: Task.async(function* (aParentId, aURL, aTitle) {
+    let node = null;
+    if (PlacesUIUtils.useAsyncTransactions) {
+      node = yield PlacesUIUtils.fetchNodeLike({ url: aURL });
+    }
+    else {
+      let linkURI = makeURI(aURL);
+      let itemId = PlacesUtils.getMostRecentBookmarkForURI(linkURI);
+      if (itemId != -1) {
+        node = { itemId, uri: aURL };
+        PlacesUIUtils.completeNodeLikeObjectForItemId(node);
+      }
+    }
+
+    if (node) {
+      PlacesUIUtils.showBookmarkDialog({ action: "edit"
+                                       , type: "bookmark"
+                                       , node
+                                       }, window);
+    }
+    else {
       PlacesUIUtils.showBookmarkDialog({ action: "add"
                                        , type: "bookmark"
                                        , uri: linkURI
@@ -358,13 +506,7 @@ var PlacesCommandHook = {
                                                      , "keyword" ]
                                        }, window);
     }
-    else {
-      PlacesUIUtils.showBookmarkDialog({ action: "edit"
-                                       , type: "bookmark"
-                                       , itemId: itemId
-                                       }, window);
-    }
-  },
+  }),
 
   /**
    * List of nsIURI objects characterizing the tabs currently open in the

browser/base/content/browser.js

@@ -9,6 +9,8 @@ let Cu = Components.utils;
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource:///modules/RecentWindow.jsm");
 
+XPCOMUtils.defineLazyModuleGetter(this, "Task",
+                                  "resource://gre/modules/Task.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "CharsetMenu",
                                   "resource:///modules/CharsetMenu.jsm");
 
@@ -1047,6 +1049,12 @@ var gBrowserInit = {
     gHomeButton.updateTooltip(homeButton);
     gHomeButton.updatePersonalToolbarStyle(homeButton);
 
+    let safeMode = document.getElementById("helpSafeMode");
+    if (Services.appinfo.inSafeMode) {
+      safeMode.label = safeMode.getAttribute("stoplabel");
+      safeMode.accesskey = safeMode.getAttribute("stopaccesskey");
+    }
+
     // BiDi UI
     gBidiUI = isBidiEnabled();
     if (gBidiUI) {
@@ -6968,57 +6976,20 @@ Object.defineProperty(this, "HUDService", {
 #endif
 
 // Prompt user to restart the browser in safe mode or normally
-function restart(safeMode)
-{
-  let promptTitleString = null;
-  let promptMessageString = null;
-  let restartTextString = null;
-  if (safeMode) {
-    promptTitleString = "safeModeRestartPromptTitle";
-    promptMessageString = "safeModeRestartPromptMessage";
-    restartTextString = "safeModeRestartButton";
-  } else {
-    promptTitleString = "restartPromptTitle";
-    promptMessageString = "restartPromptMessage";
-    restartTextString = "restartButton";
-  }
-
-  let flags = Ci.nsIAppStartup.eAttemptQuit;
-
-  // Prompt the user to confirm
-  let promptTitle = gNavigatorBundle.getString(promptTitleString);
-  let brandBundle = document.getElementById("bundle_brand");
-  let brandShortName = brandBundle.getString("brandShortName");
-  let promptMessage =
-    gNavigatorBundle.getFormattedString(promptMessageString, [brandShortName]);
-  let restartText = gNavigatorBundle.getString(restartTextString);
-  let buttonFlags = (Services.prompt.BUTTON_POS_0 *
-                     Services.prompt.BUTTON_TITLE_IS_STRING) +
-                    (Services.prompt.BUTTON_POS_1 *
-                     Services.prompt.BUTTON_TITLE_CANCEL) +
-                    Services.prompt.BUTTON_POS_0_DEFAULT;
-
-  let rv = Services.prompt.confirmEx(window, promptTitle, promptMessage,
-                                     buttonFlags, restartText, null, null,
-                                     null, {});
-
-  if (rv == 0) {
-    // Notify all windows that an application quit has been requested.
-    let cancelQuit = Components.classes["@mozilla.org/supports-PRBool;1"]
-                     .createInstance(Ci.nsISupportsPRBool);
+function safeModeRestart() {
+  if (Services.appinfo.inSafeMode) {
+    let cancelQuit = Cc["@mozilla.org/supports-PRBool;1"].
+                     createInstance(Ci.nsISupportsPRBool);
     Services.obs.notifyObservers(cancelQuit, "quit-application-requested", "restart");
 
-    // Something aborted the quit process.
-    if (cancelQuit.data) {
+    if (cancelQuit.data)
       return;
-    }
 
-    if (safeMode) {    
-      Services.startup.restartInSafeMode(flags);
-    } else {
-      Services.startup.quit(flags | Ci.nsIAppStartup.eRestart);
-    }
+    Services.startup.quit(Ci.nsIAppStartup.eRestart | Ci.nsIAppStartup.eAttemptQuit);
+    return;
   }
+
+  Services.obs.notifyObservers(null, "restart-in-safe-mode", "");
 }
 
 /* duplicateTabIn duplicates tab in a place specified by the parameter |where|.

browser/base/content/test/general/browser_syncui.js

@@ -0,0 +1,110 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+let {Weave} = Cu.import("resource://services-sync/main.js", {});
+let {Notifications} = Cu.import("resource://services-sync/notifications.js", {});
+
+let stringBundle = Cc["@mozilla.org/intl/stringbundle;1"]
+                   .getService(Ci.nsIStringBundleService)
+                   .createBundle("chrome://weave/locale/services/sync.properties");
+
+function promiseObserver(topic) {
+  return new Promise(resolve => {
+    let obs = (subject, topic, data) => {
+      Services.obs.removeObserver(obs, topic);
+      resolve(subject);
+    }
+    Services.obs.addObserver(obs, topic, false);
+  });
+}
+
+add_task(function* prepare() {
+  let xps = Components.classes["@mozilla.org/weave/service;1"]
+                              .getService(Components.interfaces.nsISupports)
+                              .wrappedJSObject;
+  yield xps.whenLoaded();
+  // mock out the "_needsSetup()" function so we don't short-circuit.
+  let oldNeedsSetup = window.gSyncUI._needsSetup;
+  window.gSyncUI._needsSetup = () => false;
+  registerCleanupFunction(() => {
+    window.gSyncUI._needsSetup = oldNeedsSetup;
+    // this test leaves the tab focused which can cause browser_tabopen_reflows
+    // to fail, so re-select the URLBar.
+    gURLBar.select();
+  });
+});
+
+add_task(function* testProlongedError() {
+  let promiseNotificationAdded = promiseObserver("weave:notification:added");
+  Assert.equal(Notifications.notifications.length, 0, "start with no notifications");
+
+  // Pretend we are in the "prolonged error" state.
+  Weave.Status.sync = Weave.PROLONGED_SYNC_FAILURE;
+  Weave.Status.login = Weave.LOGIN_SUCCEEDED;
+  Services.obs.notifyObservers(null, "weave:ui:sync:error", null);
+
+  let subject = yield promiseNotificationAdded;
+  let notification = subject.wrappedJSObject.object; // sync's observer abstraction is abstract!
+  Assert.equal(notification.title, stringBundle.GetStringFromName("error.sync.title"));
+  Assert.equal(Notifications.notifications.length, 1, "exactly 1 notification");
+
+  // Now pretend we just had a successful sync - the error notification should go away.
+  let promiseNotificationRemoved = promiseObserver("weave:notification:removed");
+  Weave.Status.sync = Weave.STATUS_OK;
+  Services.obs.notifyObservers(null, "weave:ui:sync:finish", null);
+  yield promiseNotificationRemoved;
+  Assert.equal(Notifications.notifications.length, 0, "no notifications left");
+});
+
+add_task(function* testLoginError() {
+  let promiseNotificationAdded = promiseObserver("weave:notification:added");
+  Assert.equal(Notifications.notifications.length, 0, "start with no notifications");
+
+  // Pretend we are in the "prolonged error" state.
+  Weave.Status.sync = Weave.LOGIN_FAILED;
+  Weave.Status.login = Weave.LOGIN_FAILED_LOGIN_REJECTED;
+  Services.obs.notifyObservers(null, "weave:ui:sync:error", null);
+
+  let subject = yield promiseNotificationAdded;
+  let notification = subject.wrappedJSObject.object; // sync's observer abstraction is abstract!
+  Assert.equal(notification.title, stringBundle.GetStringFromName("error.login.title"));
+  Assert.equal(Notifications.notifications.length, 1, "exactly 1 notification");
+  // Now pretend we just had a successful login - the error notification should go away.
+  Weave.Status.sync = Weave.STATUS_OK;
+  Weave.Status.login = Weave.LOGIN_SUCCEEDED;
+  let promiseNotificationRemoved = promiseObserver("weave:notification:removed");
+  Services.obs.notifyObservers(null, "weave:service:login:start", null);
+  Services.obs.notifyObservers(null, "weave:service:login:finish", null);
+  yield promiseNotificationRemoved;
+  Assert.equal(Notifications.notifications.length, 0, "no notifications left");
+});
+
+function testButtonActions(startNotification, endNotification) {
+  let button = document.getElementById("sync-button");
+  Assert.ok(button, "button exists");
+  let panelbutton = document.getElementById("PanelUI-fxa-status");
+  Assert.ok(panelbutton, "panel button exists");
+
+  // pretend a sync is starting.
+  Services.obs.notifyObservers(null, startNotification, null);
+  Assert.equal(button.getAttribute("status"), "active");
+  Assert.equal(panelbutton.getAttribute("syncstatus"), "active");
+
+  Services.obs.notifyObservers(null, endNotification, null);
+  Assert.ok(!button.hasAttribute("status"));
+  Assert.ok(!panelbutton.hasAttribute("syncstatus"));
+}
+
+add_task(function* testButtonActivities() {
+  // add the Sync button to the panel so we can get it!
+  CustomizableUI.addWidgetToArea("sync-button", CustomizableUI.AREA_PANEL);
+  // check the button's functionality
+  yield PanelUI.show();
+  try {
+    testButtonActions("weave:service:login:start", "weave:service:login:finish");
+    testButtonActions("weave:service:sync:start", "weave:ui:sync:finish");
+  } finally {
+    PanelUI.hide();
+    CustomizableUI.removeWidgetFromArea("sync-button");
+  }
+});

browser/base/jar.mn

@@ -102,7 +102,7 @@ browser.jar:
         content/browser/openLocation.xul              (content/openLocation.xul)
         content/browser/safeMode.css                  (content/safeMode.css)
         content/browser/safeMode.js                   (content/safeMode.js)
-        content/browser/safeMode.xul                  (content/safeMode.xul)
+*       content/browser/safeMode.xul                  (content/safeMode.xul)
 *       content/browser/sanitize.js                   (content/sanitize.js)
 *       content/browser/sanitize.xul                  (content/sanitize.xul)
 *       content/browser/sanitizeDialog.js             (content/sanitizeDialog.js)

browser/components/nsBrowserGlue.js

@@ -192,6 +192,9 @@ BrowserGlue.prototype = {
         Services.console.logStringMessage(null); // clear the console (in case it's open)
         Services.console.reset();
         break;
+      case "restart-in-safe-mode":
+        this._onSafeModeRestart();
+        break;
       case "quit-application-requested":
         this._onQuitRequest(subject, data);
         break;
@@ -368,6 +371,7 @@ BrowserGlue.prototype = {
     os.addObserver(this, "profile-before-change", false);
     os.addObserver(this, "browser-search-engine-modified", false);
     os.addObserver(this, "browser-search-service", false);
+    os.addObserver(this, "restart-in-safe-mode", false);
   },
 
   // cleanup (called on application shutdown)
@@ -379,6 +383,7 @@ BrowserGlue.prototype = {
     os.removeObserver(this, "browser:purge-session-history");
     os.removeObserver(this, "quit-application-requested");
     os.removeObserver(this, "quit-application-granted");
+    os.removeObserver(this, "restart-in-safe-mode");
 #ifdef OBSERVE_LASTWINDOW_CLOSE_TOPICS
     os.removeObserver(this, "browser-lastwindow-close-requested");
     os.removeObserver(this, "browser-lastwindow-close-granted");
@@ -542,6 +547,33 @@ BrowserGlue.prototype = {
     }
   },
 
+  _onSafeModeRestart: function BG_onSafeModeRestart() {
+    // prompt the user to confirm
+    let strings = Services.strings.createBundle("chrome://browser/locale/browser.properties");
+    let promptTitle = strings.GetStringFromName("safeModeRestartPromptTitle");
+    let promptMessage = strings.GetStringFromName("safeModeRestartPromptMessage");
+    let restartText = strings.GetStringFromName("safeModeRestartButton");
+    let buttonFlags = (Services.prompt.BUTTON_POS_0 *
+                       Services.prompt.BUTTON_TITLE_IS_STRING) +
+                      (Services.prompt.BUTTON_POS_1 *
+                       Services.prompt.BUTTON_TITLE_CANCEL) +
+                      Services.prompt.BUTTON_POS_0_DEFAULT;
+
+    let rv = Services.prompt.confirmEx(null, promptTitle, promptMessage,
+                                       buttonFlags, restartText, null, null,
+                                       null, {});
+    if (rv != 0)
+      return;
+
+    let cancelQuit = Cc["@mozilla.org/supports-PRBool;1"]
+                       .createInstance(Ci.nsISupportsPRBool);
+    Services.obs.notifyObservers(cancelQuit, "quit-application-requested", "restart");
+
+    if (!cancelQuit.data) {
+      Services.startup.restartInSafeMode(Ci.nsIAppStartup.eAttemptQuit);
+    }
+  },
+
   _trackSlowStartup: function () {
     if (Services.startup.interrupted ||
         Services.prefs.getBoolPref("browser.slowStartup.notificationDisabled"))

browser/components/places/PlacesUIUtils.jsm

@@ -18,6 +18,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "PluralForm",
 
 XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
                                   "resource://gre/modules/PrivateBrowsingUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Task",
+                                  "resource://gre/modules/Task.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "RecentWindow",
                                   "resource:///modules/RecentWindow.jsm");
                                   
@@ -1153,6 +1155,155 @@ this.PlacesUIUtils = {
            Weave.Service.engineManager.get("tabs") &&
            Weave.Service.engineManager.get("tabs").enabled;
   },
+
+  /**
+   * WARNING TO ADDON AUTHORS: DO NOT USE THIS METHOD. IT'S LIKELY TO BE REMOVED IN A
+   * FUTURE RELEASE.
+   *
+   * Checks if a place: href represents a folder shortcut.
+   *
+   * @param queryString
+   *        the query string to check (a place: href)
+   * @return whether or not queryString represents a folder shortcut.
+   * @throws if queryString is malformed.
+   */
+  isFolderShortcutQueryString(queryString) {
+    // Based on GetSimpleBookmarksQueryFolder in nsNavHistory.cpp.
+
+    let queriesParam = { }, optionsParam = { };
+    PlacesUtils.history.queryStringToQueries(queryString,
+                                             queriesParam,
+                                             { },
+                                             optionsParam);
+    let queries = queries.value;
+    if (queries.length == 0)
+      throw new Error(`Invalid place: uri: ${queryString}`);
+    return queries.length == 1 &&
+           queries[0].folderCount == 1 &&
+           !queries[0].hasBeginTime &&
+           !queries[0].hasEndTime &&
+           !queries[0].hasDomain &&
+           !queries[0].hasURI &&
+           !queries[0].hasSearchTerms &&
+           !queries[0].tags.length == 0 &&
+           optionsParam.value.maxResults == 0;
+  },
+
+  /**
+   * WARNING TO ADDON AUTHORS: DO NOT USE THIS METHOD. IT"S LIKELY TO BE REMOVED IN A
+   * FUTURE RELEASE.
+   *
+   * Helpers for consumers of editBookmarkOverlay which don't have a node as their input.
+   * Given a partial node-like object, having at least the itemId property set, this
+   * method completes the rest of the properties necessary for initialising the edit
+   * overlay with it.
+   *
+   * @param aNodeLike
+   *        an object having at least the itemId nsINavHistoryResultNode property set,
+   *        along with any other properties available.
+   */
+  completeNodeLikeObjectForItemId(aNodeLike) {
+    if (this.useAsyncTransactions) {
+      // When async-transactions are enabled, node-likes must have
+      // bookmarkGuid set, and we cannot set it synchronously.
+      throw new Error("completeNodeLikeObjectForItemId cannot be used when " +
+                      "async transactions are enabled");
+    }
+    if (!("itemId" in aNodeLike))
+      throw new Error("itemId missing in aNodeLike");
+
+    let itemId = aNodeLike.itemId;
+    let defGetter = XPCOMUtils.defineLazyGetter.bind(XPCOMUtils, aNodeLike);
+
+    if (!("title" in aNodeLike))
+      defGetter("title", () => PlacesUtils.bookmarks.getItemTitle(itemId));
+
+    if (!("uri" in aNodeLike)) {
+      defGetter("uri", () => {
+        let uri = null;
+        try {
+          uri = PlacesUtils.bookmarks.getBookmarkURI(itemId);
+        }
+        catch(ex) { }
+        return uri ? uri.spec : "";
+      });
+    }
+
+    if (!("type" in aNodeLike)) {
+      defGetter("type", () => {
+        if (aNodeLike.uri.length > 0) {
+          if (/^place:/.test(aNodeLike.uri)) {
+            if (this.isFolderShortcutQueryString(aNodeLike.uri))
+              return Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT;
+
+            return Ci.nsINavHistoryResultNode.RESULT_TYPE_QUERY;
+          }
+
+          return Ci.nsINavHistoryResultNode.RESULT_TYPE_URI;
+        }
+
+        let itemType = PlacesUtils.bookmarks.getItemType(itemId);
+        if (itemType == PlacesUtils.bookmarks.TYPE_FOLDER)
+          return Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER;
+
+        throw new Error("Unexpected item type");
+      });
+    }
+  },
+
+  /**
+   * Helpers for consumers of editBookmarkOverlay which don't have a node as their input.
+   *
+   * Given a bookmark object for either a url bookmark or a folder, returned by
+   * Bookmarks.fetch (see Bookmark.jsm), this creates a node-like object suitable for
+   * initialising the edit overlay with it.
+   *
+   * @param aFetchInfo
+   *        a bookmark object returned by Bookmarks.fetch.
+   * @return a node-like object suitable for initialising editBookmarkOverlay.
+   * @throws if aFetchInfo is representing a separator.
+   */
+  promiseNodeLikeFromFetchInfo: Task.async(function* (aFetchInfo) {
+    if (aFetchInfo.itemType == PlacesUtils.bookmarks.TYPE_SEPARATOR)
+      throw new Error("promiseNodeLike doesn't support separators");
+
+    return Object.freeze({
+      itemId: yield PlacesUtils.promiseItemId(aFetchInfo.guid),
+      bookmarkGuid: aFetchInfo.guid,
+      title: aFetchInfo.title,
+      uri: aFetchInfo.url !== undefined ? aFetchInfo.url.href : "",
+
+      get type() {
+        if (aFetchInfo.itemType == PlacesUtils.bookmarks.TYPE_FOLDER)
+          return Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER;
+
+        if (this.uri.length == 0)
+          throw new Error("Unexpected item type");
+
+        if (/^place:/.test(this.uri)) {
+          if (this.isFolderShortcutQueryString(this.uri))
+            return Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT;
+
+          return Ci.nsINavHistoryResultNode.RESULT_TYPE_QUERY;
+        }
+
+        return Ci.nsINavHistoryResultNode.RESULT_TYPE_URI;
+      }
+    });
+  }),
+
+  /**
+   * Shortcut for calling promiseNodeLikeFromFetchInfo on the result of
+   * Bookmarks.fetch for the given guid/info object.
+   *
+   * @see promiseNodeLikeFromFetchInfo above and Bookmarks.fetch in Bookmarks.jsm.
+   */
+  fetchNodeLike: Task.async(function* (aGuidOrInfo) {
+    let info = yield PlacesUtils.bookmarks.fetch(aGuidOrInfo);
+    if (!info)
+      return null;
+    return (yield this.promiseNodeLikeFromFetchInfo(info));
+  })
 };
 
 XPCOMUtils.defineLazyServiceGetter(PlacesUIUtils, "RDF",

browser/components/places/content/bookmarkProperties.js

@@ -37,9 +37,11 @@
  *     - "edit" - for editing a bookmark item or a folder.
  *       @ type (String). Possible values:
  *         - "bookmark"
- *           @ itemId (Integer) - the id of the bookmark item.
+ *           @ node (an nsINavHistoryResultNode object) - a node representing
+ *             the bookmark.
  *         - "folder" (also applies to livemarks)
- *           @ itemId (Integer) - the id of the folder.
+ *           @ node (an nsINavHistoryResultNode object) - a node representing
+ *             the folder.
  *   @ hiddenRows (Strings array) - optional, list of rows to be hidden
  *     regardless of the item edited or added by the dialog.
  *     Possible values:
@@ -49,10 +51,7 @@
  *     - "keyword"
  *     - "tags"
  *     - "loadInSidebar"
- *     - "feedLocation"
- *     - "siteLocation"
  *     - "folderPicker" - hides both the tree and the menu.
- *   @ readOnly (Boolean) - optional, states if the panel should be read-only
  *
  * window.arguments[0].performed is set to true if any transaction has
  * been performed by the dialog.
@@ -61,6 +60,10 @@
 Components.utils.import('resource://gre/modules/XPCOMUtils.jsm');
 XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
                                   "resource://gre/modules/PrivateBrowsingUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Task",
+                                  "resource://gre/modules/Task.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PromiseUtils",
+                                  "resource://gre/modules/PromiseUtils.jsm");
 
 const BOOKMARK_ITEM = 0;
 const BOOKMARK_FOLDER = 1;
@@ -99,7 +102,6 @@ var BookmarkPropertiesPanel = {
   _defaultInsertionPoint: null,
   _hiddenRows: [],
   _batching: false,
-  _readOnly: false,
 
   /**
    * This method returns the correct label for the dialog's "accept"
@@ -148,8 +150,8 @@ var BookmarkPropertiesPanel = {
   /**
    * Determines the initial data for the item edited or added by this dialog
    */
-  _determineItemInfo: function BPP__determineItemInfo() {
-    var dialogInfo = window.arguments[0];
+  _determineItemInfo() {
+    let dialogInfo = window.arguments[0];
     this._action = dialogInfo.action == "add" ? ACTION_ADD : ACTION_EDIT;
     this._hiddenRows = dialogInfo.hiddenRows ? dialogInfo.hiddenRows : [];
     if (this._action == ACTION_ADD) {
@@ -161,11 +163,12 @@ var BookmarkPropertiesPanel = {
       if ("defaultInsertionPoint" in dialogInfo) {
         this._defaultInsertionPoint = dialogInfo.defaultInsertionPoint;
       }
-      else
+      else {
         this._defaultInsertionPoint =
           new InsertionPoint(PlacesUtils.bookmarksMenuFolderId,
                              PlacesUtils.bookmarks.DEFAULT_INDEX,
                              Ci.nsITreeView.DROP_ON);
+      }
 
       switch (dialogInfo.type) {
         case "bookmark":
@@ -232,52 +235,15 @@ var BookmarkPropertiesPanel = {
         this._description = dialogInfo.description;
     }
     else { // edit
-      NS_ASSERT("itemId" in dialogInfo);
-      this._itemId = dialogInfo.itemId;
-      this._title = PlacesUtils.bookmarks.getItemTitle(this._itemId);
-      this._readOnly = !!dialogInfo.readOnly;
-
+      this._node = dialogInfo.node;
       switch (dialogInfo.type) {
         case "bookmark":
           this._itemType = BOOKMARK_ITEM;
-
-          this._uri = PlacesUtils.bookmarks.getBookmarkURI(this._itemId);
-          // keyword
-          this._keyword = PlacesUtils.bookmarks
-                                     .getKeywordForBookmark(this._itemId);
-          // Load In Sidebar
-          this._loadInSidebar = PlacesUtils.annotations
-                                           .itemHasAnnotation(this._itemId,
-                                                              PlacesUIUtils.LOAD_IN_SIDEBAR_ANNO);
           break;
-
         case "folder":
           this._itemType = BOOKMARK_FOLDER;
-          PlacesUtils.livemarks.getLivemark({ id: this._itemId })
-            .then(aLivemark => {
-              this._itemType = LIVEMARK_CONTAINER;
-              this._feedURI = aLivemark.feedURI;
-              this._siteURI = aLivemark.siteURI;
-              this._fillEditProperties();
-
-              let acceptButton = document.documentElement.getButton("accept");
-              acceptButton.disabled = !this._inputIsValid();
-
-              let newHeight = window.outerHeight +
-                              this._element("descriptionField").boxObject.height;
-              window.resizeTo(window.outerWidth, newHeight);
-            }, () => undefined);
-
           break;
       }
-
-      // Description
-      if (PlacesUtils.annotations
-                     .itemHasAnnotation(this._itemId, PlacesUIUtils.DESCRIPTION_ANNO)) {
-        this._description = PlacesUtils.annotations
-                                       .getItemAnnotation(this._itemId,
-                                                          PlacesUIUtils.DESCRIPTION_ANNO);
-      }
     }
   },
 
@@ -303,7 +269,7 @@ var BookmarkPropertiesPanel = {
    * This method should be called by the onload of the Bookmark Properties
    * dialog to initialize the state of the panel.
    */
-  onDialogLoad: function BPP_onDialogLoad() {
+  onDialogLoad: Task.async(function* () {
     this._determineItemInfo();
 
     document.title = this._getDialogTitle();
@@ -351,11 +317,23 @@ var BookmarkPropertiesPanel = {
 
     switch (this._action) {
       case ACTION_EDIT:
-        this._fillEditProperties();
-        acceptButton.disabled = this._readOnly;
+        gEditItemOverlay.initPanel({ node: this._node
+                                   , hiddenRows: this._hiddenRows });
+        acceptButton.disabled = gEditItemOverlay.readOnly;
         break;
       case ACTION_ADD:
-        this._fillAddProperties();
+        this._node = yield this._promiseNewItem();
+        // Edit the new item
+        gEditItemOverlay.initPanel({ node: this._node
+                                   , hiddenRows: this._hiddenRows });
+
+        // Empty location field if the uri is about:blank, this way inserting a new
+        // url will be easier for the user, Accept button will be automatically
+        // disabled by the input listener until the user fills the field.
+        let locationField = this._element("locationField");
+        if (locationField.value == "about:blank")
+          locationField.value = "";
+
         // if this is an uri related dialog disable accept button until
         // the user fills an uri value.
         if (this._itemType == BOOKMARK_ITEM)
@@ -363,7 +341,12 @@ var BookmarkPropertiesPanel = {
         break;
     }
 
-    if (!this._readOnly) {
+    // Adjust the dialog size to the changes done by initPanel. This is necessary because
+    // initPanel, which shows and hides elements, may run after some async work was done
+    // here - i.e. after the DOM load event was processed.
+    window.sizeToContent();
+
+    if (!gEditItemOverlay.readOnly) {
       // Listen on uri fields to enable accept button if input is valid
       if (this._itemType == BOOKMARK_ITEM) {
         this._element("locationField")
@@ -373,14 +356,8 @@ var BookmarkPropertiesPanel = {
               .addEventListener("input", this, false);
         }
       }
-      else if (this._itemType == LIVEMARK_CONTAINER) {
-        this._element("feedLocationField")
-            .addEventListener("input", this, false);
-        this._element("siteLocationField")
-            .addEventListener("input", this, false);
-      }
     }
-  },
+  }),
 
   // nsIDOMEventListener
   handleEvent: function BPP_handleEvent(aEvent) {
@@ -388,8 +365,6 @@ var BookmarkPropertiesPanel = {
     switch (aEvent.type) {
       case "input":
         if (target.id == "editBMPanel_locationField" ||
-            target.id == "editBMPanel_feedLocationField" ||
-            target.id == "editBMPanel_siteLocationField" ||
             target.id == "editBMPanel_keywordField") {
           // Check uri fields to enable accept button if input is valid
           document.documentElement
@@ -406,41 +381,39 @@ var BookmarkPropertiesPanel = {
     }
   },
 
-  _beginBatch: function BPP__beginBatch() {
+	// Hack for implementing batched-Undo around the editBookmarkOverlay
+	// instant-apply code. For all the details see the comment above beginBatch
+	// in browser-places.js
+  _batchBlockingDeferred: null,
+  _beginBatch() {
     if (this._batching)
       return;
-
-    PlacesUtils.transactionManager.beginBatch(null);
+    if (PlacesUIUtils.useAsyncTransactions) {
+      this._batchBlockingDeferred = PromiseUtils.defer();
+      PlacesTransactions.batch(function* () {
+        yield this._batchBlockingDeferred.promise;
+      }.bind(this));
+    }
+    else {
+      PlacesUtils.transactionManager.beginBatch(null);
+    }
     this._batching = true;
   },
 
-  _endBatch: function BPP__endBatch() {
+  _endBatch() {
     if (!this._batching)
       return;
 
-    PlacesUtils.transactionManager.endBatch(false);
+    if (PlacesUIUtils.useAsyncTransactions) {
+      this._batchBlockingDeferred.resolve();
+      this._batchBlockingDeferred = null;
+    }
+    else {
+      PlacesUtils.transactionManager.endBatch(false);
+    }
     this._batching = false;
   },
 
-  _fillEditProperties: function BPP__fillEditProperties() {
-    gEditItemOverlay.initPanel(this._itemId,
-                               { hiddenRows: this._hiddenRows,
-                                 forceReadOnly: this._readOnly });
-  },
-
-  _fillAddProperties: function BPP__fillAddProperties() {
-    this._createNewItem();
-    // Edit the new item
-    gEditItemOverlay.initPanel(this._itemId,
-                               { hiddenRows: this._hiddenRows });
-    // Empty location field if the uri is about:blank, this way inserting a new
-    // url will be easier for the user, Accept button will be automatically
-    // disabled by the input listener until the user fills the field.
-    var locationField = this._element("locationField");
-    if (locationField.value == "about:blank")
-      locationField.value = "";
-  },
-
   // nsISupports
   QueryInterface: function BPP_QueryInterface(aIID) {
     if (aIID.equals(Ci.nsIDOMEventListener) ||
@@ -454,7 +427,7 @@ var BookmarkPropertiesPanel = {
     return document.getElementById("editBMPanel_" + aID);
   },
 
-  onDialogUnload: function BPP_onDialogUnload() {
+  onDialogUnload() {
     // gEditItemOverlay does not exist anymore here, so don't rely on it.
     this._mutationObserver.disconnect();
     delete this._mutationObserver;
@@ -465,13 +438,9 @@ var BookmarkPropertiesPanel = {
     // currently registered EventListener on the EventTarget has no effect.
     this._element("locationField")
         .removeEventListener("input", this, false);
-    this._element("feedLocationField")
-        .removeEventListener("input", this, false);
-    this._element("siteLocationField")
-        .removeEventListener("input", this, false);
   },
 
-  onDialogAccept: function BPP_onDialogAccept() {
+  onDialogAccept() {
     // We must blur current focused element to save its changes correctly
     document.commandDispatcher.focusedElement.blur();
     // The order here is important! We have to uninit the panel first, otherwise
@@ -481,13 +450,16 @@ var BookmarkPropertiesPanel = {
     window.arguments[0].performed = true;
   },
 
-  onDialogCancel: function BPP_onDialogCancel() {
+  onDialogCancel() {
     // The order here is important! We have to uninit the panel first, otherwise
     // changes done as part of Undo may change the panel contents and by
     // that force it to commit more transactions.
     gEditItemOverlay.uninitPanel(true);
     this._endBatch();
-    PlacesUtils.transactionManager.undoTransaction();
+    if (PlacesUIUtils.useAsyncTransactions)
+      PlacesTransactions.undo().catch(Components.utils.reportError);
+    else
+      PlacesUtils.transactionManager.undoTransaction();
     window.arguments[0].performed = false;
   },
 
@@ -593,15 +565,14 @@ var BookmarkPropertiesPanel = {
    */
   _getTransactionsForURIList: function BPP__getTransactionsForURIList() {
     var transactions = [];
-    for (var i = 0; i < this._URIs.length; ++i) {
-      var uri = this._URIs[i];
-      var title = this._getURITitleFromHistory(uri);
-      var createTxn = new PlacesCreateBookmarkTransaction(uri, -1, 
+    for (let uri of this._URIs) {
+      let title = this._getURITitleFromHistory(uri);
+      let createTxn = new PlacesCreateBookmarkTransaction(uri, -1,
                                                           PlacesUtils.bookmarks.DEFAULT_INDEX,
                                                           title);
       transactions.push(createTxn);
     }
-    return transactions; 
+    return transactions;
   },
 
   /**
@@ -634,9 +605,6 @@ var BookmarkPropertiesPanel = {
                                                aContainer, aIndex);
   },
 
-  /**
-   * Dialog-accept code-path for creating a new item (any type)
-   */
   _createNewItem: function BPP__getCreateItemTransaction() {
     var [container, index] = this._getInsertionPointDetails();
     var txn;
@@ -647,12 +615,93 @@ var BookmarkPropertiesPanel = {
         break;
       case LIVEMARK_CONTAINER:
         txn = this._getCreateNewLivemarkTransaction(container, index);
-        break;      
+        break;
       default: // BOOKMARK_ITEM
         txn = this._getCreateNewBookmarkTransaction(container, index);
     }
 
     PlacesUtils.transactionManager.doTransaction(txn);
     this._itemId = PlacesUtils.bookmarks.getIdForItemAt(container, index);
-  }
+
+    return Object.freeze({
+      itemId: this._itemId,
+      get bookmarkGuid() {
+        throw new Error("Node-like bookmarkGuid getter called even though " +
+                        "async transactions are disabled");
+      },
+      title: this._title,
+      uri: this._uri ? this._uri.spec : "",
+      type: this._itemType == BOOKMARK_ITEM ?
+              Ci.nsINavHistoryResultNode.RESULT_TYPE_URI :
+              Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER
+    });
+  },
+
+  _promiseNewItem: Task.async(function* () {
+    if (!PlacesUIUtils.useAsyncTransactions)
+      return this._createNewItem();
+
+    let txnFunc =
+      { [BOOKMARK_FOLDER]: PlacesTransactions.NewFolder,
+        [LIVEMARK_CONTAINER]: PlacesTransactions.NewLivemark,
+        [BOOKMARK_ITEM]: PlacesTransactions.NewBookmark
+      }[this._itemType];
+
+    let [containerId, index] = this._getInsertionPointDetails();
+    let parentGuid = yield PlacesUtils.promiseItemGuid(containerId);
+    let annotations = [];
+    if (this._description) {
+      annotations.push({ name: PlacesUIUtils.DESCRIPTION_ANNO
+                       , value: this._description });
+    }
+    if (this._loadInSidebar) {
+      annotations.push({ name: PlacesUIUtils.LOAD_IN_SIDEBAR_ANNO
+                       , value: true });
+    }
+
+    let itemGuid;
+    let info = { parentGuid, index, title: this._title, annotations };
+    if (this._itemType == BOOKMARK_ITEM) {
+      info.url = this._uri;
+      if (this._keyword)
+        info.keyword = this._keyword;
+      if (this._postData)
+        info.postData = this._postData;
+
+      if (this._charSet && !PrivateBrowsingUtils.isWindowPrivate(window))
+        PlacesUtils.setCharsetForURI(this._uri, this._charSet);
+
+      itemGuid = yield PlacesTransactions.NewBookmark(info).transact();
+    }
+    else if (this._itemType == LIVEMARK_CONTAINER) {
+      info.feedUrl = this._feedURI;
+      if (this._siteURI)
+        info.siteUrl = this._siteURI;
+
+      itemGuid = yield PlacesTransactions.NewLivemark(info).transact();
+    }
+    else if (this._itemType == BOOKMARK_FOLDER) {
+      itemGuid = yield PlacesTransactions.NewFolder(info).transact();
+      for (let uri of this._URIs) {
+        let placeInfo = yield PlacesUtils.promisePlaceInfo(uri);
+        let title = placeInfo ? placeInfo.title : "";
+        yield PlacesTransactions.transact({ parentGuid: itemGuid, uri, title });
+      }
+    }
+    else {
+      throw new Error(`unexpected value for _itemType:  ${this._itemType}`);
+    }
+
+    this._itemGuid = itemGuid;
+    this._itemId = yield PlacesUtils.promiseItemId(itemGuid);
+    return Object.freeze({
+      itemId: this._itemId,
+      bookmarkGuid: this._itemGuid,
+      title: this._title,
+      uri: this._uri ? this._uri.spec : "",
+      type: this._itemType == BOOKMARK_ITEM ?
+              Ci.nsINavHistoryResultNode.RESULT_TYPE_URI :
+              Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER
+    });
+  })
 };

browser/components/places/content/controller.js

@@ -659,27 +659,13 @@ PlacesController.prototype = {
   /**
    * Opens the bookmark properties for the selected URI Node.
    */
-  showBookmarkPropertiesForSelection:
-  function PC_showBookmarkPropertiesForSelection() {
-    var node = this._view.selectedNode;
+  showBookmarkPropertiesForSelection() {
+    let node = this._view.selectedNode;
     if (!node)
       return;
 
-    var itemType = PlacesUtils.nodeIsFolder(node) ||
-                   PlacesUtils.nodeIsTagQuery(node) ? "folder" : "bookmark";
-    var concreteId = PlacesUtils.getConcreteItemId(node);
-    var isRootItem = PlacesUtils.isRootItem(concreteId);
-    var itemId = node.itemId;
-    if (isRootItem || PlacesUtils.nodeIsTagQuery(node)) {
-      // If this is a root or the Tags query we use the concrete itemId to catch
-      // the correct title for the node.
-      itemId = concreteId;
-    }
-
     PlacesUIUtils.showBookmarkDialog({ action: "edit"
-                                     , type: itemType
-                                     , itemId: itemId
-                                     , readOnly: isRootItem
+				     , node
                                      , hiddenRows: [ "folderPicker" ]
                                      }, window.top);
   },

browser/components/places/content/editBookmarkOverlay.xul

@@ -32,10 +32,9 @@
           <label value="&editBookmarkOverlay.name.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.name.accesskey;"
-                 control="editBMPanel_namePicker"
-                 observes="paneElementsBroadcaster"/>
+                 control="editBMPanel_namePicker"/>
           <textbox id="editBMPanel_namePicker"
-                   observes="paneElementsBroadcaster"/>
+                   onchange="gEditItemOverlay.onNamePickerChange();"/>
         </row>
 
         <row id="editBMPanel_locationRow"
@@ -48,33 +47,7 @@
                  observes="paneElementsBroadcaster"/>
           <textbox id="editBMPanel_locationField"
                    class="uri-element"
-                   observes="paneElementsBroadcaster"/>
-        </row>
-
-        <row id="editBMPanel_feedLocationRow"
-             align="center"
-             collapsed="true">
-          <label value="&editBookmarkOverlay.feedLocation.label;"
-                 class="editBMPanel_rowLabel"
-                 accesskey="&editBookmarkOverlay.feedLocation.accesskey;"
-                 control="editBMPanel_feedLocationField"
-                 observes="paneElementsBroadcaster"/>
-          <textbox id="editBMPanel_feedLocationField"
-                   class="uri-element"
-                   observes="paneElementsBroadcaster"/>
-        </row>
-
-        <row id="editBMPanel_siteLocationRow"
-             align="center"
-             collapsed="true">
-          <label value="&editBookmarkOverlay.siteLocation.label;"
-                 class="editBMPanel_rowLabel"
-                 accesskey="&editBookmarkOverlay.siteLocation.accesskey;"
-                 control="editBMPanel_siteLocationField"
-                 observes="paneElementsBroadcaster"/>
-          <textbox id="editBMPanel_siteLocationField"
-                   class="uri-element"
-                   observes="paneElementsBroadcaster"/>
+                   onchange="gEditItemOverlay.onLocationFieldChange();"/>
         </row>
 
         <row id="editBMPanel_folderRow"
@@ -140,7 +113,7 @@
               <button label="&editBookmarkOverlay.newFolderButton.label;"
                       id="editBMPanel_newFolderButton"
                       accesskey="&editBookmarkOverlay.newFolderButton.accesskey;"
-                      oncommand="gEditItemOverlay.newFolder();"/>
+                      oncommand="gEditItemOverlay.newFolder().catch(Components.utils.reportError);"/>
             </hbox>
           </vbox>
         </row>
@@ -163,7 +136,8 @@
                      tabscrolling="true"
                      showcommentcolumn="true"
                      observes="paneElementsBroadcaster"
-                     placeholder="&editBookmarkOverlay.tagsEmptyDesc.label;"/>
+                     placeholder="&editBookmarkOverlay.tagsEmptyDesc.label;"
+                     onchange="gEditItemOverlay.onTagsFieldChange();"/>
             <button id="editBMPanel_tagsSelectorExpander"
                     class="expander-down"
                     tooltiptext="&editBookmarkOverlay.tagsExpanderDown.tooltip;"
@@ -193,7 +167,7 @@
                  control="editBMPanel_keywordField"
                  observes="paneElementsBroadcaster"/>
           <textbox id="editBMPanel_keywordField"
-                   observes="paneElementsBroadcaster"/>
+                   onchange="gEditItemOverlay.onKeywordFieldChange();"/>
         </row>
 
         <row id="editBMPanel_descriptionRow"
@@ -206,7 +180,8 @@
                  observes="paneElementsBroadcaster"/>
           <textbox id="editBMPanel_descriptionField"
                    multiline="true"
-                   observes="paneElementsBroadcaster"/>
+                   rows="4"
+                   onchange="gEditItemOverlay.onDescriptionFieldChange();"/>
         </row>
       </rows>
     </grid>

browser/components/places/content/places.js

@@ -608,7 +608,8 @@ var PlacesOrganizer = {
     // Make sure the infoBox UI is visible if we need to use it, we hide it
     // below when we don't.
     infoBox.hidden = false;
-    var aSelectedNode = aNodeList.length == 1 ? aNodeList[0] : null;
+    let selectedNode = aNodeList.length == 1 ? aNodeList[0] : null;
+
     // If a textbox within a panel is focused, force-blur it so its contents
     // are saved
     if (gEditItemOverlay.itemId != -1) {
@@ -620,12 +621,12 @@ var PlacesOrganizer = {
 
       // don't update the panel if we are already editing this node unless we're
       // in multi-edit mode
-      if (aSelectedNode) {
-        var concreteId = PlacesUtils.getConcreteItemId(aSelectedNode);
-        var nodeIsSame = gEditItemOverlay.itemId == aSelectedNode.itemId ||
+      if (selectedNode) {
+        var concreteId = PlacesUtils.getConcreteItemId(selectedNode);
+        var nodeIsSame = gEditItemOverlay.itemId == selectedNode.itemId ||
                          gEditItemOverlay.itemId == concreteId ||
-                         (aSelectedNode.itemId == -1 && gEditItemOverlay.uri &&
-                          gEditItemOverlay.uri == aSelectedNode.uri);
+                         (selectedNode.itemId == -1 && gEditItemOverlay.uri &&
+                          gEditItemOverlay.uri == selectedNode.uri);
         if (nodeIsSame && detailsDeck.selectedIndex == 1 &&
             !gEditItemOverlay.multiEdit)
           return;
@@ -635,70 +636,54 @@ var PlacesOrganizer = {
     // Clean up the panel before initing it again.
     gEditItemOverlay.uninitPanel(false);
 
-    if (aSelectedNode && !PlacesUtils.nodeIsSeparator(aSelectedNode)) {
+    if (selectedNode && !PlacesUtils.nodeIsSeparator(selectedNode)) {
       detailsDeck.selectedIndex = 1;
       // Using the concrete itemId is arguably wrong.  The bookmarks API
       // does allow setting properties for folder shortcuts as well, but since
       // the UI does not distinct between the couple, we better just show
       // the concrete item properties for shortcuts to root nodes.
-      var concreteId = PlacesUtils.getConcreteItemId(aSelectedNode);
+      var concreteId = PlacesUtils.getConcreteItemId(selectedNode);
       var isRootItem = concreteId != -1 && PlacesUtils.isRootItem(concreteId);
       var readOnly = isRootItem ||
-                     aSelectedNode.parent.itemId == PlacesUIUtils.leftPaneFolderId;
+                     selectedNode.parent.itemId == PlacesUIUtils.leftPaneFolderId;
       var useConcreteId = isRootItem ||
-                          PlacesUtils.nodeIsTagQuery(aSelectedNode);
+                          PlacesUtils.nodeIsTagQuery(selectedNode);
       var itemId = -1;
       if (concreteId != -1 && useConcreteId)
         itemId = concreteId;
-      else if (aSelectedNode.itemId != -1)
-        itemId = aSelectedNode.itemId;
+      else if (selectedNode.itemId != -1)
+        itemId = selectedNode.itemId;
       else
-        itemId = PlacesUtils._uri(aSelectedNode.uri);
+        itemId = PlacesUtils._uri(selectedNode.uri);
 
-      gEditItemOverlay.initPanel(itemId, { hiddenRows: ["folderPicker"]
-                                         , forceReadOnly: readOnly
-                                         , titleOverride: aSelectedNode.title
-                                         });
+      gEditItemOverlay.initPanel({ node: selectedNode
+                                 , hiddenRows: ["folderPicker"] });
 
-      // Dynamically generated queries, like history date containers, have
-      // itemId !=0 and do not exist in history.  For them the panel is
-      // read-only, but empty, since it can't get a valid title for the object.
-      // In such a case we force the title using the selectedNode one, for UI
-      // polishness.
-      if (aSelectedNode.itemId == -1 &&
-          (PlacesUtils.nodeIsDay(aSelectedNode) ||
-           PlacesUtils.nodeIsHost(aSelectedNode)))
-        gEditItemOverlay._element("namePicker").value = aSelectedNode.title;
-
-      this._detectAndSetDetailsPaneMinimalState(aSelectedNode);
+      this._detectAndSetDetailsPaneMinimalState(selectedNode);
     }
-    else if (!aSelectedNode && aNodeList[0]) {
-      var itemIds = [];
-      for (var i = 0; i < aNodeList.length; i++) {
-        if (!PlacesUtils.nodeIsBookmark(aNodeList[i]) &&
-            !PlacesUtils.nodeIsURI(aNodeList[i])) {
-          detailsDeck.selectedIndex = 0;
-          var selectItemDesc = document.getElementById("selectItemDescription");
-          var itemsCountLabel = document.getElementById("itemsCountText");
-          selectItemDesc.hidden = false;
-          itemsCountLabel.value =
-            PlacesUIUtils.getPluralString("detailsPane.itemsCountLabel",
-                                          aNodeList.length, [aNodeList.length]);
-          infoBox.hidden = true;
-          return;
-        }
-        itemIds[i] = aNodeList[i].itemId != -1 ? aNodeList[i].itemId :
-                     PlacesUtils._uri(aNodeList[i].uri);
+    else if (!selectedNode && aNodeList[0]) {
+      if (aNodeList.every(PlacesUtils.nodeIsURI)) {
+        let uris = [for (node of aNodeList) PlacesUtils._uri(node.uri)];
+        detailsDeck.selectedIndex = 1;
+        gEditItemOverlay.initPanel({ uris
+                                   , hiddenRows: ["folderPicker",
+                                                  "loadInSidebar",
+                                                  "location",
+                                                  "keyword",
+                                                  "description",
+                                                  "name"]});
+        this._detectAndSetDetailsPaneMinimalState(selectedNode);
+      }
+      else {
+        detailsDeck.selectedIndex = 0;
+        let selectItemDesc = document.getElementById("selectItemDescription");
+        let itemsCountLabel = document.getElementById("itemsCountText");
+        selectItemDesc.hidden = false;
+        itemsCountLabel.value =
+          PlacesUIUtils.getPluralString("detailsPane.itemsCountLabel",
+                                        aNodeList.length, [aNodeList.length]);
+        infoBox.hidden = true;
       }
-      detailsDeck.selectedIndex = 1;
-      gEditItemOverlay.initPanel(itemIds,
-                                 { hiddenRows: ["folderPicker",
-                                                "loadInSidebar",
-                                                "location",
-                                                "keyword",
-                                                "description",
-                                                "name"]});
-      this._detectAndSetDetailsPaneMinimalState(aSelectedNode);
     }
     else {
       detailsDeck.selectedIndex = 0;

browser/components/places/content/treeView.js

@@ -1739,8 +1739,13 @@ PlacesTreeView.prototype = {
     // We may only get here if the cell is editable.
     let node = this._rows[aRow];
     if (node.title != aText) {
-      let txn = new PlacesEditItemTitleTransaction(node.itemId, aText);
-      PlacesUtils.transactionManager.doTransaction(txn);
+      if (!PlacesUIUtils.useAsyncTransactions) {
+        let txn = new PlacesEditItemTitleTransaction(node.itemId, aText);
+        PlacesUtils.transactionManager.doTransaction(txn);
+        return;
+      }
+      PlacesTransactions.EditTitle({ guid: node.bookmarkGuid, title: aText })
+                        .transact().catch(Cu.reportError);
     }
   },
 

browser/locales/en-US/chrome/browser/baseMenuOverlay.dtd

@@ -21,6 +21,8 @@
 <!ENTITY helpMac.commandkey       "?">
 <!ENTITY helpSafeMode.label       "Restart in Safe Mode…">
 <!ENTITY helpSafeMode.accesskey   "R">
+<!ENTITY helpSafeMode.stop.label       "Restart with Add-ons Enabled">
+<!ENTITY helpSafeMode.stop.accesskey   "R">
 
 <!ENTITY helpTroubleshootingInfo.label      "Troubleshooting Information">
 <!ENTITY helpTroubleshootingInfo.accesskey  "T">

toolkit/components/places/Database.cpp

@@ -735,6 +735,13 @@ Database::InitSchema(bool* aDatabaseMigrated)
 
       // Firefox 37 uses schema version 26.
 
+      if (currentSchemaVersion < 27) {
+        rv = MigrateV27Up();
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
+
+      // Firefox 38 uses schema version 27.
+
       // Schema Upgrades must add migration code here.
 
       rv = UpdateBookmarkRootTitles();
@@ -801,6 +808,8 @@ Database::InitSchema(bool* aDatabaseMigrated)
     // moz_keywords.
     rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_KEYWORDS);
     NS_ENSURE_SUCCESS(rv, rv);
+    rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_KEYWORDS_PLACEPOSTDATA);
+    NS_ENSURE_SUCCESS(rv, rv);
 
     // moz_favicons.
     rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_FAVICONS);
@@ -951,11 +960,18 @@ Database::InitTempTriggers()
   rv = mMainConn->ExecuteSimpleSQL(CREATE_PLACES_AFTERUPDATE_TYPED_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = mMainConn->ExecuteSimpleSQL(CREATE_FOREIGNCOUNT_AFTERDELETE_TRIGGER);
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERDELETE_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
-  rv = mMainConn->ExecuteSimpleSQL(CREATE_FOREIGNCOUNT_AFTERINSERT_TRIGGER);
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERINSERT_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
-  rv = mMainConn->ExecuteSimpleSQL(CREATE_FOREIGNCOUNT_AFTERUPDATE_TRIGGER);
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERUPDATE_TRIGGER);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_KEYWORDS_FOREIGNCOUNT_AFTERDELETE_TRIGGER);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_KEYWORDS_FOREIGNCOUNT_AFTERINSERT_TRIGGER);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_KEYWORDS_FOREIGNCOUNT_AFTERUPDATE_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -1487,6 +1503,66 @@ Database::MigrateV26Up() {
   return NS_OK;
 }
 
+nsresult
+Database::MigrateV27Up() {
+  MOZ_ASSERT(NS_IsMainThread());
+
+  // Change keywords store, moving their relation from bookmarks to urls.
+  nsCOMPtr<mozIStorageStatement> stmt;
+  nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
+    "SELECT place_id FROM moz_keywords"
+  ), getter_AddRefs(stmt));
+  if (NS_FAILED(rv)) {
+    // Even if these 2 columns have a unique constraint, we allow NULL values
+    // for backwards compatibility. NULL never breaks a unique constraint.
+    rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+      "ALTER TABLE moz_keywords ADD COLUMN place_id INTEGER"));
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+      "ALTER TABLE moz_keywords ADD COLUMN post_data TEXT"));
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_KEYWORDS_PLACEPOSTDATA);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  // Associate keywords with uris.  A keyword could be associated to multiple
+  // bookmarks uris, or multiple keywords could be associated to the same uri.
+  // The new system only allows multiple uris per keyword, provided they have
+  // a different post_data value.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "INSERT OR REPLACE INTO moz_keywords (id, keyword, place_id, post_data) "
+    "SELECT k.id, k.keyword, h.id, MAX(a.content) "
+    "FROM moz_places h "
+    "JOIN moz_bookmarks b ON b.fk = h.id "
+    "JOIN moz_keywords k ON k.id = b.keyword_id "
+    "LEFT JOIN moz_items_annos a ON a.item_id = b.id "
+    "LEFT JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id "
+                                   "AND n.name = 'bookmarkProperties/POSTData'"
+    "WHERE k.place_id ISNULL "
+    "GROUP BY keyword"));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Remove any keyword that points to a non-existing place id.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DELETE FROM moz_keywords "
+    "WHERE NOT EXISTS (SELECT 1 FROM moz_places WHERE id = moz_keywords.place_id)"));
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "UPDATE moz_bookmarks SET keyword_id = NULL "
+    "WHERE NOT EXISTS (SELECT 1 FROM moz_keywords WHERE id = moz_bookmarks.keyword_id)"));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Adjust foreign_count for all the rows.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "UPDATE moz_places SET foreign_count = "
+    "(SELECT count(*) FROM moz_bookmarks WHERE fk = moz_places.id) + "
+    "(SELECT count(*) FROM moz_keywords WHERE place_id = moz_places.id) "
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return NS_OK;
+}
+
 void
 Database::Shutdown()
 {

toolkit/components/places/Database.h

@@ -16,7 +16,7 @@
 
 // This is the schema version. Update it at any schema change and add a
 // corresponding migrateVxx method below.
-#define DATABASE_SCHEMA_VERSION 26
+#define DATABASE_SCHEMA_VERSION 27
 
 // Fired after Places inited.
 #define TOPIC_PLACES_INIT_COMPLETE "places-init-complete"
@@ -273,6 +273,7 @@ protected:
   nsresult MigrateV24Up();
   nsresult MigrateV25Up();
   nsresult MigrateV26Up();
+  nsresult MigrateV27Up();
 
   nsresult UpdateBookmarkRootTitles();
 

toolkit/components/places/PlacesDBUtils.jsm

@@ -474,23 +474,6 @@ this.PlacesDBUtils = {
     fixOrphanItems.params["tagsGuid"] = PlacesUtils.bookmarks.tagsGuid;
     cleanupStatements.push(fixOrphanItems);
 
-    // D.5 fix wrong keywords
-    let fixInvalidKeywords = DBConn.createAsyncStatement(
-      `UPDATE moz_bookmarks SET keyword_id = NULL WHERE guid NOT IN (
-         :rootGuid, :menuGuid, :toolbarGuid, :unfiledGuid, :tagsGuid  /* skip roots */
-       ) AND id IN (
-         SELECT id FROM moz_bookmarks b
-         WHERE keyword_id NOT NULL
-           AND NOT EXISTS
-             (SELECT id FROM moz_keywords WHERE id = b.keyword_id LIMIT 1)
-       )`);
-    fixInvalidKeywords.params["rootGuid"] = PlacesUtils.bookmarks.rootGuid;
-    fixInvalidKeywords.params["menuGuid"] = PlacesUtils.bookmarks.menuGuid;
-    fixInvalidKeywords.params["toolbarGuid"] = PlacesUtils.bookmarks.toolbarGuid;
-    fixInvalidKeywords.params["unfiledGuid"] = PlacesUtils.bookmarks.unfiledGuid;
-    fixInvalidKeywords.params["tagsGuid"] = PlacesUtils.bookmarks.tagsGuid;
-    cleanupStatements.push(fixInvalidKeywords);
-
     // D.6 fix wrong item types
     //     Folders and separators should not have an fk.
     //     If they have a valid fk convert them to bookmarks. Later in D.9 we
@@ -681,7 +664,7 @@ this.PlacesDBUtils = {
       `DELETE FROM moz_keywords WHERE id IN (
          SELECT id FROM moz_keywords k
          WHERE NOT EXISTS
-           (SELECT id FROM moz_bookmarks WHERE keyword_id = k.id LIMIT 1)
+           (SELECT 1 FROM moz_places h WHERE k.place_id = h.id)
        )`);
     cleanupStatements.push(deleteUnusedKeywords);
 

toolkit/components/places/PlacesUtils.jsm

@@ -386,6 +386,23 @@ this.PlacesUtils = {
     return aNode.itemId;
   },
 
+  /**
+   * Gets the concrete item-guid for the given node. For everything but folder
+   * shortcuts, this is just node.bookmarkGuid.  For folder shortcuts, this is
+   * node.targetFolderGuid (see nsINavHistoryService.idl for the semantics).
+   *
+   * @param aNode
+   *        a result node.
+   * @return the concrete item-guid for aNode.
+   * @note unlike getConcreteItemId, this doesn't allow retrieving the guid of a
+   *       ta container.
+   */
+  getConcreteItemGuid(aNode) {
+    if (aNode.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT)
+      return asQuery(aNode).targetFolderGuid;
+    return aNode.bookmarkGuid;
+  },
+
   /**
    * Reverse a host based on the moz_places algorithm, that is reverse the host
    * string and add a trailing period.  For example "google.com" becomes
@@ -814,13 +831,26 @@ this.PlacesUtils = {
    *   @param aBookmarkId
    *   @returns string of POST data
    */
-  setPostDataForBookmark: function PU_setPostDataForBookmark(aBookmarkId, aPostData) {
-    const annos = this.annotations;
-    if (aPostData)
-      annos.setItemAnnotation(aBookmarkId, this.POST_DATA_ANNO, aPostData, 
-                              0, Ci.nsIAnnotationService.EXPIRE_NEVER);
-    else if (annos.itemHasAnnotation(aBookmarkId, this.POST_DATA_ANNO))
-      annos.removeItemAnnotation(aBookmarkId, this.POST_DATA_ANNO);
+  setPostDataForBookmark(aBookmarkId, aPostData) {
+    // For now we don't have a unified API to create a keyword with postData,
+    // thus here we can just try to complete a keyword that should already exist
+    // without any post data.
+    let nullPostDataFragment = aPostData ? "AND post_data ISNULL" : "";
+    let stmt = PlacesUtils.history.DBConnection.createStatement(
+      `UPDATE moz_keywords SET post_data = :post_data
+       WHERE id = (SELECT k.id FROM moz_keywords k
+                   JOIN moz_bookmarks b ON b.fk = k.place_id
+                   WHERE b.id = :item_id
+                   ${nullPostDataFragment}
+                   LIMIT 1)`);
+    stmt.params.item_id = aBookmarkId;
+    stmt.params.post_data = aPostData;
+    try {
+      stmt.execute();
+    }
+    finally {
+      stmt.finalize();
+    }
   },
 
   /**
@@ -828,12 +858,22 @@ this.PlacesUtils = {
    * @param aBookmarkId
    * @returns string of POST data if set for aBookmarkId. null otherwise.
    */
-  getPostDataForBookmark: function PU_getPostDataForBookmark(aBookmarkId) {
-    const annos = this.annotations;
-    if (annos.itemHasAnnotation(aBookmarkId, this.POST_DATA_ANNO))
-      return annos.getItemAnnotation(aBookmarkId, this.POST_DATA_ANNO);
-
-    return null;
+  getPostDataForBookmark(aBookmarkId) {
+    let stmt = PlacesUtils.history.DBConnection.createStatement(
+      `SELECT k.post_data
+       FROM moz_keywords k
+       JOIN moz_places h ON h.id = k.place_id
+       JOIN moz_bookmarks b ON b.fk = h.id
+       WHERE b.id = :item_id`);
+    stmt.params.item_id = aBookmarkId;
+    try {
+      if (!stmt.executeStep())
+        return null;
+      return stmt.row.post_data;
+    }
+    finally {
+      stmt.finalize();
+    }
   },
 
   /**
@@ -841,24 +881,21 @@ this.PlacesUtils = {
    * @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;
+  getURLAndPostDataForKeyword(aKeyword) {
+    let stmt = PlacesUtils.history.DBConnection.createStatement(
+      `SELECT h.url, k.post_data
+       FROM moz_keywords k
+       JOIN moz_places h ON h.id = k.place_id
+       WHERE k.keyword = :keyword`);
+    stmt.params.keyword = aKeyword;
     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];
+      if (!stmt.executeStep())
+        return [ null, null ];
+      return [ stmt.row.url, stmt.row.post_data ];
+    }
+    finally {
+      stmt.finalize();
+    }
   },
 
   /**
@@ -3064,7 +3101,7 @@ PlacesSortFolderByNameTransaction.prototype = {
     let callback = {
       _self: this,
       runBatched: function() {
-        for (item in this._self._oldOrder)
+        for (let item in this._self._oldOrder)
           PlacesUtils.bookmarks.setItemIndex(item, this._self._oldOrder[item]);
       }
     };

toolkit/components/places/UnifiedComplete.js

@@ -152,24 +152,20 @@ const SQL_ADAPTIVE_QUERY =
 const SQL_KEYWORD_QUERY =
   `/* do not warn (bug 487787) */
    SELECT :query_type,
-     (SELECT REPLACE(url, '%s', :query_string) FROM moz_places WHERE id = b.fk)
-     AS search_url, h.title,
+          REPLACE(h.url, '%s', :query_string) AS search_url, h.title,
      IFNULL(f.url, (SELECT f.url
                     FROM moz_places
                     JOIN moz_favicons f ON f.id = favicon_id
-                    WHERE rev_host = (SELECT rev_host FROM moz_places WHERE id = b.fk)
+                    WHERE rev_host = h.rev_host
                     ORDER BY frecency DESC
                     LIMIT 1)
            ),
-     1, b.title, NULL, h.visit_count, h.typed, IFNULL(h.id, b.fk),
-     t.open_count, h.frecency
+     1, NULL, NULL, h.visit_count, h.typed, h.id, t.open_count, h.frecency
    FROM moz_keywords k
-   JOIN moz_bookmarks b ON b.keyword_id = k.id
-   LEFT JOIN moz_places h ON h.url = search_url
+   JOIN moz_places h ON k.place_id = h.id
    LEFT JOIN moz_favicons f ON f.id = h.favicon_id
    LEFT JOIN moz_openpages_temp t ON t.url = search_url
-   WHERE LOWER(k.keyword) = LOWER(:keyword)
-   ORDER BY h.frecency DESC`;
+   WHERE k.keyword = LOWER(:keyword)`;
 
 function hostQuery(conditions = "") {
   let query =
@@ -1241,24 +1237,13 @@ Search.prototype = {
     let title = bookmarkTitle || historyTitle;
 
     if (queryType == QUERYTYPE_KEYWORD) {
+      match.style = "keyword";
       if (this._enableActions) {
-        match.style = "keyword";
         url = makeActionURL("keyword", {
           url: escapedURL,
           input: this._originalSearchString,
         });
         action = "keyword";
-      } else {
-        // If we do not have a title, then we must have a keyword, so let the UI
-        // know it is a keyword.  Otherwise, we found an exact page match, so just
-        // show the page like a regular result.  Because the page title is likely
-        // going to be more specific than the bookmark title (keyword title).
-        if (!historyTitle) {
-          match.style = "keyword"
-        }
-        else {
-          title = historyTitle;
-        }
       }
     }
 

toolkit/components/places/nsINavBookmarksService.idl

@@ -222,7 +222,7 @@ interface nsINavBookmarkObserver : nsISupports
  * folders.  A URI in history can be contained in one or more such folders.
  */
 
-[scriptable, uuid(b0f9a80a-d7f0-4421-8513-444125f0d828)]
+[scriptable, uuid(24533891-afa6-4663-b72d-3143d03f1b04)]
 interface nsINavBookmarksService : nsISupports
 {
   /**
@@ -536,12 +536,6 @@ interface nsINavBookmarksService : nsISupports
    */
   void setKeywordForBookmark(in long long aItemId, in AString aKeyword);
 
-  /**
-   * Retrieves the keyword for the given URI. Will be void string
-   * (null in JS) if no such keyword is found.
-   */
-  AString getKeywordForURI(in nsIURI aURI);
-
   /**
    * Retrieves the keyword for the given bookmark. Will be void string
    * (null in JS) if no such keyword is found.

toolkit/components/places/nsNavBookmarks.cpp

@@ -19,8 +19,6 @@
 
 #include "GeckoProfiler.h"
 
-#define BOOKMARKS_TO_KEYWORDS_INITIAL_CACHE_LENGTH 32
-
 using namespace mozilla;
 
 // These columns sit to the right of the kGetInfoIndex_* columns.
@@ -40,25 +38,6 @@ PLACES_FACTORY_SINGLETON_IMPLEMENTATION(nsNavBookmarks, gBookmarksService)
 
 namespace {
 
-struct keywordSearchData
-{
-  int64_t itemId;
-  nsString keyword;
-};
-
-PLDHashOperator
-SearchBookmarkForKeyword(nsTrimInt64HashKey::KeyType aKey,
-                         const nsString aValue,
-                         void* aUserArg)
-{
-  keywordSearchData* data = reinterpret_cast<keywordSearchData*>(aUserArg);
-  if (data->keyword.Equals(aValue)) {
-    data->itemId = aKey;
-    return PL_DHASH_STOP;
-  }
-  return PL_DHASH_NEXT;
-}
-
 template<typename Method, typename DataType>
 class AsyncGetBookmarksForURI : public AsyncStatementCallback
 {
@@ -143,8 +122,6 @@ nsNavBookmarks::nsNavBookmarks()
   , mCanNotify(false)
   , mCacheObservers("bookmark-observers")
   , mBatching(false)
-  , mBookmarkToKeywordHash(BOOKMARKS_TO_KEYWORDS_INITIAL_CACHE_LENGTH)
-  , mBookmarkToKeywordHashInitialized(false)
 {
   NS_ASSERTION(!gBookmarksService,
                "Attempting to create two instances of the service!");
@@ -646,11 +623,12 @@ nsNavBookmarks::RemoveItem(int64_t aItemId)
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
-    rv = UpdateKeywordsHashForRemovedBookmark(aItemId);
-    NS_ENSURE_SUCCESS(rv, rv);
-
     // A broken url should not interrupt the removal process.
-    (void)NS_NewURI(getter_AddRefs(uri), bookmark.url);
+    rv = NS_NewURI(getter_AddRefs(uri), bookmark.url);
+    if (NS_SUCCEEDED(rv)) {
+      rv = UpdateKeywordsForRemovedBookmark(bookmark);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
   }
 
   NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
@@ -1108,8 +1086,14 @@ nsNavBookmarks::RemoveFolderChildren(int64_t aFolderId)
   rv = SetItemDateInternal(LAST_MODIFIED, folder.id, RoundedPRNow());
   NS_ENSURE_SUCCESS(rv, rv);
 
-  for (uint32_t i = 0; i < folderChildrenArray.Length(); i++) {
+  rv = transaction.Commit();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Call observers in reverse order to serve children before their parent.
+  for (int32_t i = folderChildrenArray.Length() - 1; i >= 0; --i) {
     BookmarkData& child = folderChildrenArray[i];
+
+    nsCOMPtr<nsIURI> uri;
     if (child.type == TYPE_BOOKMARK) {
       // If not a tag, recalculate frecency for this entry, since it changed.
       if (child.grandParentId != mTagsRoot) {
@@ -1119,21 +1103,12 @@ nsNavBookmarks::RemoveFolderChildren(int64_t aFolderId)
         NS_ENSURE_SUCCESS(rv, rv);
       }
 
-      rv = UpdateKeywordsHashForRemovedBookmark(child.id);
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-  }
-
-  rv = transaction.Commit();
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // Call observers in reverse order to serve children before their parent.
-  for (int32_t i = folderChildrenArray.Length() - 1; i >= 0; --i) {
-    BookmarkData& child = folderChildrenArray[i];
-    nsCOMPtr<nsIURI> uri;
-    if (child.type == TYPE_BOOKMARK) {
       // A broken url should not interrupt the removal process.
-      (void)NS_NewURI(getter_AddRefs(uri), child.url);
+      rv = NS_NewURI(getter_AddRefs(uri), child.url);
+      if (NS_SUCCEEDED(rv)) {
+        rv = UpdateKeywordsForRemovedBookmark(child);
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
     }
 
     NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
@@ -2264,39 +2239,69 @@ nsNavBookmarks::SetItemIndex(int64_t aItemId, int32_t aNewIndex)
 
 
 nsresult
-nsNavBookmarks::UpdateKeywordsHashForRemovedBookmark(int64_t aItemId)
+nsNavBookmarks::UpdateKeywordsForRemovedBookmark(const BookmarkData& aBookmark)
 {
-  nsAutoString keyword;
-  if (NS_SUCCEEDED(GetKeywordForBookmark(aItemId, keyword)) &&
-      !keyword.IsEmpty()) {
-    nsresult rv = EnsureKeywordsHash();
+  // If there are no keywords for this URI, there's nothing to do.
+  nsCOMPtr<nsIURI> uri;
+  nsresult rv = NS_NewURI(getter_AddRefs(uri), aBookmark.url);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsTArray<nsString> keywords;
+  {
+    nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
+      "SELECT keyword FROM moz_keywords WHERE place_id = :place_id "
+    );
+    NS_ENSURE_STATE(stmt);
+    mozStorageStatementScoper scoper(stmt);
+    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("place_id"), aBookmark.placeId);
     NS_ENSURE_SUCCESS(rv, rv);
-    mBookmarkToKeywordHash.Remove(aItemId);
 
-    // If the keyword is unused, remove it from the database.
-    keywordSearchData searchData;
-    searchData.keyword.Assign(keyword);
-    searchData.itemId = -1;
-    mBookmarkToKeywordHash.EnumerateRead(SearchBookmarkForKeyword, &searchData);
-    if (searchData.itemId == -1) {
-      nsCOMPtr<mozIStorageAsyncStatement> stmt = mDB->GetAsyncStatement(
-        "DELETE FROM moz_keywords "
-        "WHERE keyword = :keyword "
-        "AND NOT EXISTS ( "
-          "SELECT id "
-          "FROM moz_bookmarks "
-          "WHERE keyword_id = moz_keywords.id "
-        ")"
-      );
-      NS_ENSURE_STATE(stmt);
-
-      rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
-      NS_ENSURE_SUCCESS(rv, rv);
-      nsCOMPtr<mozIStoragePendingStatement> pendingStmt;
-      rv = stmt->ExecuteAsync(nullptr, getter_AddRefs(pendingStmt));
+    bool hasMore;
+    while (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
+      nsAutoString keyword;
+      rv = stmt->GetString(0, keyword);
       NS_ENSURE_SUCCESS(rv, rv);
+      keywords.AppendElement(keyword);
     }
   }
+
+  if (keywords.Length() == 0) {
+    // This uri has no keywords associated, so there's nothing to do.
+    return NS_OK;
+  }
+
+  // If the uri is not bookmarked anymore, we can remove its keywords.
+  nsTArray<BookmarkData> bookmarks;
+  rv = GetBookmarksForURI(uri, bookmarks);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (bookmarks.Length() == 0) {
+    for (uint32_t i = 0; i < keywords.Length(); ++i) {
+      nsString keyword = keywords[i];
+
+      nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
+        "DELETE FROM moz_keywords WHERE keyword = :keyword "
+      );
+      NS_ENSURE_STATE(stmt);
+      mozStorageStatementScoper scoper(stmt);
+      rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->Execute();
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+
+    NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+                     nsINavBookmarkObserver,
+                     OnItemChanged(aBookmark.id,
+                                   NS_LITERAL_CSTRING("keyword"),
+                                   false,
+                                   EmptyCString(),
+                                   aBookmark.lastModified,
+                                   TYPE_BOOKMARK,
+                                   aBookmark.parentId,
+                                   aBookmark.guid,
+                                   aBookmark.parentGuid));
+  }
+
   return NS_OK;
 }
 
@@ -2311,121 +2316,163 @@ nsNavBookmarks::SetKeywordForBookmark(int64_t aBookmarkId,
   BookmarkData bookmark;
   nsresult rv = FetchItemInfo(aBookmarkId, bookmark);
   NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = EnsureKeywordsHash();
+  nsCOMPtr<nsIURI> uri;
+  rv = NS_NewURI(getter_AddRefs(uri), bookmark.url);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Shortcuts are always lowercased internally.
   nsAutoString keyword(aUserCasedKeyword);
   ToLowerCase(keyword);
 
-  // Check if bookmark was already associated to a keyword.
-  nsAutoString oldKeyword;
-  rv = GetKeywordForBookmark(bookmark.id, oldKeyword);
-  NS_ENSURE_SUCCESS(rv, rv);
+  // The same URI can be associated to more than one keyword, provided the post
+  // data differs.  Check if there are already keywords associated to this uri.
+  nsTArray<nsString> oldKeywords;
+  {
+    nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
+      "SELECT keyword FROM moz_keywords WHERE place_id = :place_id"
+    );
+    NS_ENSURE_STATE(stmt);
+    mozStorageStatementScoper scoper(stmt);
+    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("place_id"), bookmark.placeId);
+    NS_ENSURE_SUCCESS(rv, rv);
 
-  // Trying to set the same value or to remove a nonexistent keyword is a no-op.
-  if (keyword.Equals(oldKeyword) || (keyword.IsEmpty() && oldKeyword.IsEmpty()))
+    bool hasMore;
+    while (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
+      nsString oldKeyword;
+      rv = stmt->GetString(0, oldKeyword);
+      NS_ENSURE_SUCCESS(rv, rv);
+      oldKeywords.AppendElement(oldKeyword);
+    }
+  }
+
+  // Trying to remove a non-existent keyword is a no-op.
+  if (keyword.IsEmpty() && oldKeywords.Length() == 0) {
     return NS_OK;
-
-  mozStorageTransaction transaction(mDB->MainConn(), false);
-
-  nsCOMPtr<mozIStorageStatement> updateBookmarkStmt = mDB->GetStatement(
-    "UPDATE moz_bookmarks "
-    "SET keyword_id = (SELECT id FROM moz_keywords WHERE keyword = :keyword), "
-        "lastModified = :date "
-    "WHERE id = :item_id "
-  );
-  NS_ENSURE_STATE(updateBookmarkStmt);
-  mozStorageStatementScoper updateBookmarkScoper(updateBookmarkStmt);
+  }
 
   if (keyword.IsEmpty()) {
-    // Remove keyword association from the hash.
-    mBookmarkToKeywordHash.Remove(bookmark.id);
-    rv = updateBookmarkStmt->BindNullByName(NS_LITERAL_CSTRING("keyword"));
+    // We are removing the existing keywords.
+    for (uint32_t i = 0; i < oldKeywords.Length(); ++i) {
+      nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
+        "DELETE FROM moz_keywords WHERE keyword = :old_keyword"
+      );
+      NS_ENSURE_STATE(stmt);
+      mozStorageStatementScoper scoper(stmt);
+      rv = stmt->BindStringByName(NS_LITERAL_CSTRING("old_keyword"),
+                                  oldKeywords[i]);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->Execute();
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+
+    nsTArray<BookmarkData> bookmarks;
+    rv = GetBookmarksForURI(uri, bookmarks);
+    NS_ENSURE_SUCCESS(rv, rv);
+    for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
+      NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+                       nsINavBookmarkObserver,
+                       OnItemChanged(bookmarks[i].id,
+                                     NS_LITERAL_CSTRING("keyword"),
+                                     false,
+                                     EmptyCString(),
+                                     bookmarks[i].lastModified,
+                                     TYPE_BOOKMARK,
+                                     bookmarks[i].parentId,
+                                     bookmarks[i].guid,
+                                     bookmarks[i].parentGuid));
+    }
+
+    return NS_OK;
   }
-   else {
-    // We are associating bookmark to a new keyword. Create a new keyword
-    // record if needed.
-    nsCOMPtr<mozIStorageStatement> newKeywordStmt = mDB->GetStatement(
-      "INSERT OR IGNORE INTO moz_keywords (keyword) VALUES (:keyword)"
+
+  // A keyword can only be associated to a single URI.  Check if the requested
+  // keyword was already associated, in such a case we will need to notify about
+  // the change.
+  nsCOMPtr<nsIURI> oldUri;
+  {
+    nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
+      "SELECT url "
+      "FROM moz_keywords "
+      "JOIN moz_places h ON h.id = place_id "
+      "WHERE keyword = :keyword"
     );
-    NS_ENSURE_STATE(newKeywordStmt);
-    mozStorageStatementScoper newKeywordScoper(newKeywordStmt);
-
-    rv = newKeywordStmt->BindStringByName(NS_LITERAL_CSTRING("keyword"),
-                                          keyword);
-    NS_ENSURE_SUCCESS(rv, rv);
-    rv = newKeywordStmt->Execute();
+    NS_ENSURE_STATE(stmt);
+    mozStorageStatementScoper scoper(stmt);
+    rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    // Add new keyword association to the hash, removing the old one if needed.
-    if (!oldKeyword.IsEmpty())
-      mBookmarkToKeywordHash.Remove(bookmark.id);
-    mBookmarkToKeywordHash.Put(bookmark.id, keyword);
-    rv = updateBookmarkStmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
+    bool hasMore;
+    if (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
+      nsAutoCString spec;
+      rv = stmt->GetUTF8String(0, spec);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = NS_NewURI(getter_AddRefs(oldUri), spec);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
   }
-  NS_ENSURE_SUCCESS(rv, rv);
-  bookmark.lastModified = RoundedPRNow();
-  rv = updateBookmarkStmt->BindInt64ByName(NS_LITERAL_CSTRING("date"),
-                                           bookmark.lastModified);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = updateBookmarkStmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"),
-                                           bookmark.id);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = updateBookmarkStmt->Execute();
-  NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = transaction.Commit();
-  NS_ENSURE_SUCCESS(rv, rv);
+  // If another uri is using the new keyword, we must update the keyword entry.
+  // Note we cannot use INSERT OR REPLACE cause it wouldn't invoke the delete
+  // trigger.
+  nsCOMPtr<mozIStorageStatement> stmt;
+  if (oldUri) {
+    // In both cases, notify about the change.
+    nsTArray<BookmarkData> bookmarks;
+    rv = GetBookmarksForURI(oldUri, bookmarks);
+    NS_ENSURE_SUCCESS(rv, rv);
+    for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
+      NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+                       nsINavBookmarkObserver,
+                       OnItemChanged(bookmarks[i].id,
+                                     NS_LITERAL_CSTRING("keyword"),
+                                     false,
+                                     EmptyCString(),
+                                     bookmarks[i].lastModified,
+                                     TYPE_BOOKMARK,
+                                     bookmarks[i].parentId,
+                                     bookmarks[i].guid,
+                                     bookmarks[i].parentGuid));
+    }
 
-  NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
-                   nsINavBookmarkObserver,
-                   OnItemChanged(bookmark.id,
-                                 NS_LITERAL_CSTRING("keyword"),
-                                 false,
-                                 NS_ConvertUTF16toUTF8(keyword),
-                                 bookmark.lastModified,
-                                 bookmark.type,
-                                 bookmark.parentId,
-                                 bookmark.guid,
-                                 bookmark.parentGuid));
-
-  return NS_OK;
-}
-
-
-NS_IMETHODIMP
-nsNavBookmarks::GetKeywordForURI(nsIURI* aURI, nsAString& aKeyword)
-{
-  PLACES_WARN_DEPRECATED();
-
-  NS_ENSURE_ARG(aURI);
-  aKeyword.Truncate(0);
-
-  nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
-    "SELECT k.keyword "
-    "FROM moz_places h "
-    "JOIN moz_bookmarks b ON b.fk = h.id "
-    "JOIN moz_keywords k ON k.id = b.keyword_id "
-    "WHERE h.url = :page_url "
-  );
+    stmt = mDB->GetStatement(
+      "UPDATE moz_keywords SET place_id = :place_id WHERE keyword = :keyword"
+    );
+    NS_ENSURE_STATE(stmt);
+  }
+  else {
+    stmt = mDB->GetStatement(
+      "INSERT INTO moz_keywords (keyword, place_id) "
+      "VALUES (:keyword, :place_id)"
+    );
+  }
   NS_ENSURE_STATE(stmt);
   mozStorageStatementScoper scoper(stmt);
 
-  nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), aURI);
+  rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("place_id"), bookmark.placeId);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  bool hasMore = false;
-  rv = stmt->ExecuteStep(&hasMore);
-  if (NS_FAILED(rv) || !hasMore) {
-    aKeyword.SetIsVoid(true);
-    return NS_OK; // not found: return void keyword string
+  // In both cases, notify about the change.
+  nsTArray<BookmarkData> bookmarks;
+  rv = GetBookmarksForURI(uri, bookmarks);
+  NS_ENSURE_SUCCESS(rv, rv);
+  for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
+    NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+                     nsINavBookmarkObserver,
+                     OnItemChanged(bookmarks[i].id,
+                                   NS_LITERAL_CSTRING("keyword"),
+                                   false,
+                                   NS_ConvertUTF16toUTF8(keyword),
+                                   bookmarks[i].lastModified,
+                                   TYPE_BOOKMARK,
+                                   bookmarks[i].parentId,
+                                   bookmarks[i].guid,
+                                   bookmarks[i].parentGuid));
   }
 
-  // found, get the keyword
-  rv = stmt->GetString(0, aKeyword);
-  NS_ENSURE_SUCCESS(rv, rv);
   return NS_OK;
 }
 
@@ -2436,17 +2483,34 @@ nsNavBookmarks::GetKeywordForBookmark(int64_t aBookmarkId, nsAString& aKeyword)
   NS_ENSURE_ARG_MIN(aBookmarkId, 1);
   aKeyword.Truncate(0);
 
-  nsresult rv = EnsureKeywordsHash();
+  // We can have multiple keywords for the same uri, here we'll just return the
+  // last created one.
+  nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(NS_LITERAL_CSTRING(
+    "SELECT k.keyword "
+    "FROM moz_bookmarks b "
+    "JOIN moz_keywords k ON k.place_id = b.fk "
+    "WHERE b.id = :item_id "
+    "ORDER BY k.ROWID DESC "
+    "LIMIT 1"
+  ));
+  NS_ENSURE_STATE(stmt);
+  mozStorageStatementScoper scoper(stmt);
+
+  nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"),
+                             aBookmarkId);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsAutoString keyword;
-  if (!mBookmarkToKeywordHash.Get(aBookmarkId, &keyword)) {
-    aKeyword.SetIsVoid(true);
-  }
-  else {
-    aKeyword.Assign(keyword);
+  bool hasMore;
+  if (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
+    nsAutoString keyword;
+    rv = stmt->GetString(0, keyword);
+    NS_ENSURE_SUCCESS(rv, rv);
+    aKeyword = keyword;
+    return NS_OK;
   }
 
+  aKeyword.SetIsVoid(true);
+
   return NS_OK;
 }
 
@@ -2463,51 +2527,27 @@ nsNavBookmarks::GetURIForKeyword(const nsAString& aUserCasedKeyword,
   nsAutoString keyword(aUserCasedKeyword);
   ToLowerCase(keyword);
 
-  nsresult rv = EnsureKeywordsHash();
-  NS_ENSURE_SUCCESS(rv, rv);
+  nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(NS_LITERAL_CSTRING(
+    "SELECT h.url "
+    "FROM moz_places h "
+    "JOIN moz_keywords k ON k.place_id = h.id "
+    "WHERE k.keyword = :keyword"
+  ));
+  NS_ENSURE_STATE(stmt);
+  mozStorageStatementScoper scoper(stmt);
 
-  keywordSearchData searchData;
-  searchData.keyword.Assign(keyword);
-  searchData.itemId = -1;
-  mBookmarkToKeywordHash.EnumerateRead(SearchBookmarkForKeyword, &searchData);
-
-  if (searchData.itemId == -1) {
-    // Not found.
-    return NS_OK;
-  }
-
-  rv = GetBookmarkURI(searchData.itemId, aURI);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  return NS_OK;
-}
-
-
-nsresult
-nsNavBookmarks::EnsureKeywordsHash() {
-  if (mBookmarkToKeywordHashInitialized) {
-    return NS_OK;
-  }
-  mBookmarkToKeywordHashInitialized = true;
-
-  nsCOMPtr<mozIStorageStatement> stmt;
-  nsresult rv = mDB->MainConn()->CreateStatement(NS_LITERAL_CSTRING(
-    "SELECT b.id, k.keyword "
-    "FROM moz_bookmarks b "
-    "JOIN moz_keywords k ON k.id = b.keyword_id "
-  ), getter_AddRefs(stmt));
+  nsresult rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
   NS_ENSURE_SUCCESS(rv, rv);
 
   bool hasMore;
-  while (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
-    int64_t itemId;
-    rv = stmt->GetInt64(0, &itemId);
+  if (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
+    nsAutoCString spec;
+    rv = stmt->GetUTF8String(0, spec);
     NS_ENSURE_SUCCESS(rv, rv);
-    nsAutoString keyword;
-    rv = stmt->GetString(1, keyword);
+    nsCOMPtr<nsIURI> uri;
+    rv = NS_NewURI(getter_AddRefs(uri), spec);
     NS_ENSURE_SUCCESS(rv, rv);
-
-    mBookmarkToKeywordHash.Put(itemId, keyword);
+    uri.forget(aURI);
   }
 
   return NS_OK;

toolkit/components/places/nsNavBookmarks.h

@@ -420,21 +420,13 @@ private:
   // Note: this is only tracking bookmarks batches, not history ones.
   bool mBatching;
 
-  /**
-   * Always call EnsureKeywordsHash() and check it for errors before actually
-   * using the hash.  Internal keyword methods are already doing that.
-   */
-  nsresult EnsureKeywordsHash();
-  nsDataHashtable<nsTrimInt64HashKey, nsString> mBookmarkToKeywordHash;
-  bool mBookmarkToKeywordHashInitialized;
-
   /**
    * This function must be called every time a bookmark is removed.
    *
    * @param aURI
    *        Uri to test.
    */
-  nsresult UpdateKeywordsHashForRemovedBookmark(int64_t aItemId);
+  nsresult UpdateKeywordsForRemovedBookmark(const BookmarkData& aBookmark);
 };
 
 #endif // nsNavBookmarks_h_

toolkit/components/places/nsPlacesAutoComplete.js

@@ -414,24 +414,20 @@ function nsPlacesAutoComplete()
   XPCOMUtils.defineLazyGetter(this, "_keywordQuery", function() {
     return this._db.createAsyncStatement(
       `/* do not warn (bug 487787) */
-       SELECT
-       (SELECT REPLACE(url, '%s', :query_string) FROM moz_places WHERE id = b.fk)
-       AS search_url, h.title,
+       SELECT REPLACE(h.url, '%s', :query_string) AS search_url, h.title,
        IFNULL(f.url, (SELECT f.url
                       FROM moz_places
                       JOIN moz_favicons f ON f.id = favicon_id
-                      WHERE rev_host = (SELECT rev_host FROM moz_places WHERE id = b.fk)
+                      WHERE rev_host = h.rev_host
                       ORDER BY frecency DESC
                       LIMIT 1)
-       ), 1, b.title, NULL, h.visit_count, h.typed, IFNULL(h.id, b.fk),
+       ), 1, NULL, NULL, h.visit_count, h.typed, h.id,
        :query_type, t.open_count
        FROM moz_keywords k
-       JOIN moz_bookmarks b ON b.keyword_id = k.id
-       LEFT JOIN moz_places h ON h.url = search_url
+       JOIN moz_places h ON k.place_id = h.id
        LEFT JOIN moz_favicons f ON f.id = h.favicon_id
        LEFT JOIN moz_openpages_temp t ON t.url = search_url
-       WHERE LOWER(k.keyword) = LOWER(:keyword)
-       ORDER BY h.frecency DESC`
+       WHERE k.keyword = LOWER(:keyword)`
     );
   });
 

toolkit/components/places/nsPlacesIndexes.h

@@ -121,4 +121,13 @@
     "guid_uniqueindex", "moz_favicons", "guid", "UNIQUE" \
   )
 
+/**
+ * moz_keywords
+ */
+
+#define CREATE_IDX_MOZ_KEYWORDS_PLACEPOSTDATA \
+  CREATE_PLACES_IDX( \
+    "placepostdata_uniqueindex", "moz_keywords", "place_id, post_data", "UNIQUE" \
+  )
+
 #endif // nsPlacesIndexes_h__

toolkit/components/places/nsPlacesTables.h

@@ -121,6 +121,8 @@
   "CREATE TABLE moz_keywords (" \
     "  id INTEGER PRIMARY KEY AUTOINCREMENT" \
     ", keyword TEXT UNIQUE" \
+    ", place_id INTEGER" \
+    ", post_data TEXT" \
   ")" \
 )
 

toolkit/components/places/nsPlacesTriggers.h

@@ -175,7 +175,7 @@
   "END" \
 )
 
-#define CREATE_FOREIGNCOUNT_AFTERDELETE_TRIGGER NS_LITERAL_CSTRING( \
+#define CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERDELETE_TRIGGER NS_LITERAL_CSTRING( \
   "CREATE TEMP TRIGGER moz_bookmarks_foreign_count_afterdelete_trigger " \
   "AFTER DELETE ON moz_bookmarks FOR EACH ROW " \
   "BEGIN " \
@@ -185,7 +185,7 @@
   "END" \
 )
 
-#define CREATE_FOREIGNCOUNT_AFTERINSERT_TRIGGER NS_LITERAL_CSTRING( \
+#define CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERINSERT_TRIGGER NS_LITERAL_CSTRING( \
   "CREATE TEMP TRIGGER moz_bookmarks_foreign_count_afterinsert_trigger " \
   "AFTER INSERT ON moz_bookmarks FOR EACH ROW " \
   "BEGIN " \
@@ -195,7 +195,7 @@
   "END" \
 )
 
-#define CREATE_FOREIGNCOUNT_AFTERUPDATE_TRIGGER NS_LITERAL_CSTRING( \
+#define CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERUPDATE_TRIGGER NS_LITERAL_CSTRING( \
   "CREATE TEMP TRIGGER moz_bookmarks_foreign_count_afterupdate_trigger " \
   "AFTER UPDATE OF fk ON moz_bookmarks FOR EACH ROW " \
   "BEGIN " \
@@ -207,4 +207,38 @@
     "WHERE id = OLD.fk;" \
   "END" \
 )
+
+#define CREATE_KEYWORDS_FOREIGNCOUNT_AFTERDELETE_TRIGGER NS_LITERAL_CSTRING( \
+  "CREATE TEMP TRIGGER moz_keywords_foreign_count_afterdelete_trigger " \
+  "AFTER DELETE ON moz_keywords FOR EACH ROW " \
+  "BEGIN " \
+    "UPDATE moz_places " \
+    "SET foreign_count = foreign_count - 1 " \
+    "WHERE id = OLD.place_id;" \
+  "END" \
+)
+
+#define CREATE_KEYWORDS_FOREIGNCOUNT_AFTERINSERT_TRIGGER NS_LITERAL_CSTRING( \
+  "CREATE TEMP TRIGGER moz_keyords_foreign_count_afterinsert_trigger " \
+  "AFTER INSERT ON moz_keywords FOR EACH ROW " \
+  "BEGIN " \
+    "UPDATE moz_places " \
+    "SET foreign_count = foreign_count + 1 " \
+    "WHERE id = NEW.place_id;" \
+  "END" \
+)
+
+#define CREATE_KEYWORDS_FOREIGNCOUNT_AFTERUPDATE_TRIGGER NS_LITERAL_CSTRING( \
+  "CREATE TEMP TRIGGER moz_keywords_foreign_count_afterupdate_trigger " \
+  "AFTER UPDATE OF place_id ON moz_keywords FOR EACH ROW " \
+  "BEGIN " \
+    "UPDATE moz_places " \
+    "SET foreign_count = foreign_count + 1 " \
+    "WHERE id = NEW.place_id; " \
+    "UPDATE moz_places " \
+    "SET foreign_count = foreign_count - 1 " \
+    "WHERE id = OLD.place_id; " \
+  "END" \
+)
+
 #endif // __nsPlacesTriggers_h__

toolkit/components/places/tests/autocomplete/test_keyword_search.js

@@ -43,13 +43,13 @@ let kTitles = [
 // Add the keyword bookmark
 addPageBook(0, 0, 1, [], keyKey);
 // Add in the "fake pages" for keyword searches
-gPages[1] = [1,1];
-gPages[2] = [2,1];
-gPages[3] = [3,1];
-gPages[4] = [4,1];
+gPages[1] = [1,0];
+gPages[2] = [2,0];
+gPages[3] = [3,0];
+gPages[4] = [4,0];
 // Add a page into history
 addPageBook(5, 0);
-gPages[6] = [6,1];
+gPages[6] = [6,0];
 
 // Provide for each test: description; search terms; array of gPages indices of
 // pages that should match; optional function to be run before the test
@@ -68,14 +68,4 @@ let gTests = [
    keyKey, [6]],
   ["6: Keyword without query (with space)",
    keyKey + " ", [6]],
-
-  // This adds a second keyword so anything after this will match 2 keywords
-  ["7: Two keywords matched",
-   keyKey + " twoKey", [8,9],
-   function() {
-     // Add the keyword search as well as search results
-     addPageBook(7, 0, 1, [], keyKey);
-     gPages[8] = [8,1];
-     gPages[9] = [9,1];
-   }]
 ];

toolkit/components/places/tests/bookmarks/test_keywords.js

@@ -1,169 +1,307 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
+const URI1 = NetUtil.newURI("http://test1.mozilla.org/");
+const URI2 = NetUtil.newURI("http://test2.mozilla.org/");
+const URI3 = NetUtil.newURI("http://test3.mozilla.org/");
 
-function check_bookmark_keyword(aItemId, aKeyword)
-{
-  let keyword = aKeyword ? aKeyword.toLowerCase() : null;
-  do_check_eq(PlacesUtils.bookmarks.getKeywordForBookmark(aItemId),
-              keyword);
-}
-
-function check_uri_keyword(aURI, aKeyword)
-{
-  let keyword = aKeyword ? aKeyword.toLowerCase() : null;
+function check_keyword(aURI, aKeyword) {
+  if (aKeyword)
+    aKeyword = aKeyword.toLowerCase();
 
   for (let bm of PlacesUtils.getBookmarksForURI(aURI)) {
-    let kid = PlacesUtils.bookmarks.getKeywordForBookmark(bm);
-    if (kid && !keyword) {
-      Assert.ok(false, `${aURI.spec} should not have a keyword`);
-    } else if (keyword && kid == keyword) {
-      Assert.equal(kid, keyword, "Found the keyword");
-      break;
+    let keyword = PlacesUtils.bookmarks.getKeywordForBookmark(bm);
+    if (keyword && !aKeyword) {
+      throw(`${aURI.spec} should not have a keyword`);
+    } else if (aKeyword && keyword == aKeyword) {
+      Assert.equal(keyword, aKeyword);
     }
   }
 
   if (aKeyword) {
-    // This API can't tell which uri the user wants, so it returns a random one.
-    let re = /http:\/\/test[0-9]\.mozilla\.org/;
-    let url = PlacesUtils.bookmarks.getURIForKeyword(aKeyword).spec;
-    do_check_true(re.test(url));
+    let uri = PlacesUtils.bookmarks.getURIForKeyword(aKeyword);
+    Assert.equal(uri.spec, aURI.spec);
     // Check case insensitivity.
-    url = PlacesUtils.bookmarks.getURIForKeyword(aKeyword.toUpperCase()).spec
-    do_check_true(re.test(url));
+    uri = PlacesUtils.bookmarks.getURIForKeyword(aKeyword.toUpperCase());
+    Assert.equal(uri.spec, aURI.spec);
   }
 }
 
-function check_orphans()
-{
-  let stmt = DBConn().createStatement(
-    `SELECT id FROM moz_keywords k WHERE NOT EXISTS (
-        SELECT id FROM moz_bookmarks WHERE keyword_id = k.id
-     )`
-  );
-  try {
-    do_check_false(stmt.executeStep());
-  } finally {
-    stmt.finalize();
-  }
-
-  print("Check there are no orphan database entries");
-  stmt = DBConn().createStatement(
-    `SELECT b.id FROM moz_bookmarks b
-     LEFT JOIN moz_keywords k ON b.keyword_id = k.id
-     WHERE keyword_id NOTNULL AND k.id ISNULL`
-  );
-  try {
-    do_check_false(stmt.executeStep());
-  } finally {
-    stmt.finalize();
-  }
+function check_orphans() {
+  let db = yield PlacesUtils.promiseDBConnection();
+  let rows = yield db.executeCached(
+    `SELECT id FROM moz_keywords k
+     WHERE NOT EXISTS (SELECT 1 FROM moz_places WHERE id = k.place_id)
+    `);
+  Assert.equal(rows.length, 0);
 }
 
-const URIS = [
-  uri("http://test1.mozilla.org/"),
-  uri("http://test2.mozilla.org/"),
-];
+function expectNotifications() {
+  let notifications = [];
+  let observer = new Proxy(NavBookmarkObserver, {
+    get(target, name) {
+      if (name == "check") {
+        PlacesUtils.bookmarks.removeObserver(observer);
+        return expectedNotifications =>
+          Assert.deepEqual(notifications, expectedNotifications);
+      }
 
-add_test(function test_addBookmarkWithKeyword()
-{
-  check_uri_keyword(URIS[0], null);
+      if (name.startsWith("onItemChanged")) {
+        return (id, prop, isAnno, val, lastMod, itemType, parentId, guid, parentGuid) => {
+          if (prop != "keyword")
+            return;
+          let args = Array.from(arguments, arg => {
+            if (arg && arg instanceof Ci.nsIURI)
+              return new URL(arg.spec);
+            if (arg && typeof(arg) == "number" && arg >= Date.now() * 1000)
+              return new Date(parseInt(arg/1000));
+            return arg;
+          });
+          notifications.push({ name: name, arguments: args });
+        }
+      }
+
+      return target[name];
+    }
+  });
+  PlacesUtils.bookmarks.addObserver(observer, false);
+  return observer;
+}
+
+add_task(function test_invalid_input() {
+  Assert.throws(() => PlacesUtils.bookmarks.getURIForKeyword(null),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.bookmarks.getURIForKeyword(""),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.bookmarks.getKeywordForBookmark(null),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.bookmarks.getKeywordForBookmark(0),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.bookmarks.setKeywordForBookmark(null, "k"),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.bookmarks.setKeywordForBookmark(0, "k"),
+                /NS_ERROR_ILLEGAL_VALUE/);
+});
+
+add_task(function test_addBookmarkAndKeyword() {
+  check_keyword(URI1, null);
+  let fc = yield foreign_count(URI1);
+  let observer = expectNotifications();
 
   let itemId =
     PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URIS[0],
+                                         URI1,
                                          PlacesUtils.bookmarks.DEFAULT_INDEX,
                                          "test");
+
   PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword");
-  check_bookmark_keyword(itemId, "keyword");
-  check_uri_keyword(URIS[0], "keyword");
+  let bookmark = yield PlacesUtils.bookmarks.fetch({ url: URI1 });
+  observer.check([ { name: "onItemChanged",
+                     arguments: [ itemId, "keyword", false, "keyword",
+                                  bookmark.lastModified, bookmark.type,
+                                  (yield PlacesUtils.promiseItemId(bookmark.parentGuid)),
+                                  bookmark.guid, bookmark.parentGuid ] }
+                 ]);
+  yield PlacesTestUtils.promiseAsyncUpdates();
 
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
-    check_orphans();
-    run_next_test();
-  });
+  check_keyword(URI1, "keyword");
+  Assert.equal((yield foreign_count(URI1)), fc + 2); // + 1 bookmark + 1 keyword
+
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  yield check_orphans();
 });
 
-add_test(function test_addBookmarkToURIHavingKeyword()
-{
+add_task(function test_addBookmarkToURIHavingKeyword() {
+  // The uri has already a keyword.
+  check_keyword(URI1, "keyword");
+  let fc = yield foreign_count(URI1);
+
   let itemId =
     PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URIS[0],
+                                         URI1,
                                          PlacesUtils.bookmarks.DEFAULT_INDEX,
                                          "test");
-  // The uri has a keyword, but this specific bookmark has not.
-  check_bookmark_keyword(itemId, null);
-  check_uri_keyword(URIS[0], "keyword");
+  check_keyword(URI1, "keyword");
+  Assert.equal((yield foreign_count(URI1)), fc + 1); // + 1 bookmark
 
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
-    check_orphans();
-    run_next_test();
-  });
+  PlacesUtils.bookmarks.removeItem(itemId);
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  check_orphans();
 });
 
-add_test(function test_addSameKeywordToOtherURI()
-{
+add_task(function test_sameKeywordDifferentURI() {
+  let fc1 = yield foreign_count(URI1);
+  let fc2 = yield foreign_count(URI2);
+  let observer = expectNotifications();
+
   let itemId =
     PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URIS[1],
+                                         URI2,
                                          PlacesUtils.bookmarks.DEFAULT_INDEX,
                                          "test2");
-  check_bookmark_keyword(itemId, null);
-  check_uri_keyword(URIS[1], null);
+  check_keyword(URI1, "keyword");
+  check_keyword(URI2, null);
 
   PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "kEyWoRd");
-  check_bookmark_keyword(itemId, "kEyWoRd");
-  check_uri_keyword(URIS[1], "kEyWoRd");
 
-  // Check case insensitivity.
-  check_uri_keyword(URIS[0], "kEyWoRd");
-  check_bookmark_keyword(itemId, "keyword");
-  check_uri_keyword(URIS[1], "keyword");
-  check_uri_keyword(URIS[0], "keyword");
+  let bookmark1 = yield PlacesUtils.bookmarks.fetch({ url: URI1 });
+  let bookmark2 = yield PlacesUtils.bookmarks.fetch({ url: URI2 });
+  observer.check([ { name: "onItemChanged",
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmark1.guid)),
+                                  "keyword", false, "",
+                                  bookmark1.lastModified, bookmark1.type,
+                                  (yield PlacesUtils.promiseItemId(bookmark1.parentGuid)),
+                                  bookmark1.guid, bookmark1.parentGuid ] },
+                    { name: "onItemChanged",
+                     arguments: [ itemId, "keyword", false, "keyword",
+                                  bookmark2.lastModified, bookmark2.type,
+                                  (yield PlacesUtils.promiseItemId(bookmark2.parentGuid)),
+                                  bookmark2.guid, bookmark2.parentGuid ] }
+                 ]);
+  yield PlacesTestUtils.promiseAsyncUpdates();
 
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
-    check_orphans();
-    run_next_test();
-  });
+  // The keyword should have been "moved" to the new URI.
+  check_keyword(URI1, null);
+  Assert.equal((yield foreign_count(URI1)), fc1 - 1); // - 1 keyword
+  check_keyword(URI2, "keyword");
+  Assert.equal((yield foreign_count(URI2)), fc2 + 2); // + 1 bookmark + 1 keyword
+
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  check_orphans();
 });
 
-add_test(function test_removeBookmarkWithKeyword()
-{
+add_task(function test_sameURIDifferentKeyword() {
+  let fc = yield foreign_count(URI2);
+  let observer = expectNotifications();
+
   let itemId =
     PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URIS[1],
+                                         URI2,
+                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
+                                         "test2");
+  check_keyword(URI2, "keyword");
+
+  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword2");
+
+  let bookmarks = [];
+  yield PlacesUtils.bookmarks.fetch({ url: URI2 }, bookmark => bookmarks.push(bookmark));
+  observer.check([ { name: "onItemChanged",
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmarks[0].guid)),
+                                  "keyword", false, "keyword2",
+                                  bookmarks[0].lastModified, bookmarks[0].type,
+                                  (yield PlacesUtils.promiseItemId(bookmarks[0].parentGuid)),
+                                  bookmarks[0].guid, bookmarks[0].parentGuid ] },
+                    { name: "onItemChanged",
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmarks[1].guid)),
+                                  "keyword", false, "keyword2",
+                                  bookmarks[1].lastModified, bookmarks[1].type,
+                                  (yield PlacesUtils.promiseItemId(bookmarks[1].parentGuid)),
+                                  bookmarks[1].guid, bookmarks[1].parentGuid ] }
+                 ]);
+  yield PlacesTestUtils.promiseAsyncUpdates();
+
+  check_keyword(URI2, "keyword2");
+  Assert.equal((yield foreign_count(URI2)), fc + 2); // + 1 bookmark + 1 keyword
+
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  check_orphans();
+});
+
+add_task(function test_removeBookmarkWithKeyword() {
+  let fc = yield foreign_count(URI2);
+  let itemId =
+    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
+                                         URI2,
                                          PlacesUtils.bookmarks.DEFAULT_INDEX,
                                          "test");
-  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword");
-  check_bookmark_keyword(itemId, "keyword");
-  check_uri_keyword(URIS[1], "keyword");
 
-  // The keyword should not be removed from other bookmarks.
+   // The keyword should not be removed, since there are other bookmarks yet.
+   PlacesUtils.bookmarks.removeItem(itemId);
+
+  check_keyword(URI2, "keyword2");
+  Assert.equal((yield foreign_count(URI2)), fc); // + 1 bookmark - 1 bookmark
+
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  check_orphans();
+});
+
+add_task(function test_unsetKeyword() {
+  let fc = yield foreign_count(URI2);
+  let observer = expectNotifications();
+
+  let itemId =
+    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
+                                         URI2,
+                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
+                                         "test");
+
+  // The keyword should be removed from any bookmark.
+  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, null);
+
+  let bookmarks = [];
+  yield PlacesUtils.bookmarks.fetch({ url: URI2 }, bookmark => bookmarks.push(bookmark));
+  do_print(bookmarks.length);
+  observer.check([ { name: "onItemChanged",
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmarks[0].guid)),
+                                  "keyword", false, "",
+                                  bookmarks[0].lastModified, bookmarks[0].type,
+                                  (yield PlacesUtils.promiseItemId(bookmarks[0].parentGuid)),
+                                  bookmarks[0].guid, bookmarks[0].parentGuid ] },
+                    { name: "onItemChanged",
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmarks[1].guid)),
+                                  "keyword", false, "",
+                                  bookmarks[1].lastModified, bookmarks[1].type,
+                                  (yield PlacesUtils.promiseItemId(bookmarks[1].parentGuid)),
+                                  bookmarks[1].guid, bookmarks[1].parentGuid ] },
+                    { name: "onItemChanged",
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmarks[2].guid)),
+                                  "keyword", false, "",
+                                  bookmarks[2].lastModified, bookmarks[2].type,
+                                  (yield PlacesUtils.promiseItemId(bookmarks[2].parentGuid)),
+                                  bookmarks[2].guid, bookmarks[2].parentGuid ] }
+                 ]);
+
+  check_keyword(URI1, null);
+  check_keyword(URI2, null);
+  Assert.equal((yield foreign_count(URI2)), fc - 1); // + 1 bookmark - 2 keyword
+
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  check_orphans();
+});
+
+add_task(function test_addRemoveBookmark() {
+  let fc = yield foreign_count(URI3);
+  let observer = expectNotifications();
+
+  let itemId =
+    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
+                                         URI3,
+                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
+                                         "test3");
+
+  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword");
+  let bookmark = yield PlacesUtils.bookmarks.fetch({ url: URI3 });
+  let parentId = yield PlacesUtils.promiseItemId(bookmark.parentGuid);
   PlacesUtils.bookmarks.removeItem(itemId);
 
-  check_uri_keyword(URIS[1], "keyword");
-  check_uri_keyword(URIS[0], "keyword");
+  observer.check([ { name: "onItemChanged",
+                     arguments: [ itemId,
+                                  "keyword", false, "keyword",
+                                  bookmark.lastModified, bookmark.type,
+                                  parentId,
+                                  bookmark.guid, bookmark.parentGuid ] },
+                    { name: "onItemChanged",
+                     arguments: [ itemId,
+                                  "keyword", false, "",
+                                  bookmark.lastModified, bookmark.type,
+                                  parentId,
+                                  bookmark.guid, bookmark.parentGuid ] }
+                 ]);
 
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
-    check_orphans();
-    run_next_test();
-  });
+  check_keyword(URI3, null);
+  Assert.equal((yield foreign_count(URI3)), fc);
+
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  check_orphans();
 });
 
-add_test(function test_removeFolderWithKeywordedBookmarks()
-{
-  // Keyword should be removed as well.
-  PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId);
-
-  check_uri_keyword(URIS[1], null);
-  check_uri_keyword(URIS[0], null);
-
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
-    check_orphans();
-    run_next_test();
-  });
-});
-
-function run_test()
-{
+function run_test() {
   run_next_test();
 }

toolkit/components/places/tests/head_common.js

@@ -3,7 +3,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-const CURRENT_SCHEMA_VERSION = 26;
+const CURRENT_SCHEMA_VERSION = 27;
 const FIRST_UPGRADABLE_SCHEMA_VERSION = 11;
 
 const NS_APP_USER_PROFILE_50_DIR = "ProfD";
@@ -851,3 +851,17 @@ function checkBookmarkObject(info) {
   Assert.ok(info.lastModified >= info.dateAdded, "lastModified should never be smaller than dateAdded");
   Assert.ok(typeof info.type == "number", "type should be a number");
 }
+
+/**
+ * Reads foreign_count value for a given url.
+ */
+function* foreign_count(url) {
+  if (url instanceof Ci.nsIURI)
+    url = url.spec;
+  let db = yield PlacesUtils.promiseDBConnection();
+  let rows = yield db.executeCached(
+    `SELECT foreign_count FROM moz_places
+     WHERE url = :url
+    `, { url });
+  return rows.length == 0 ? 0 : rows[0].getResultByName("foreign_count");
+}

toolkit/components/places/tests/migration/test_current_from_v26.js

@@ -0,0 +1,75 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+add_task(function* setup() {
+  yield setupPlacesDatabase("places_v26.sqlite");
+  // Setup database contents to be migrated.
+  let path = OS.Path.join(OS.Constants.Path.profileDir, DB_FILENAME);
+  let db = yield Sqlite.openConnection({ path });
+  // Add pages.
+  yield db.execute(`INSERT INTO moz_places (url, guid)
+                    VALUES ("http://test1.com/", "test1_______")
+                         , ("http://test2.com/", "test2_______")
+                   `);
+  // Add keywords.
+  yield db.execute(`INSERT INTO moz_keywords (keyword)
+                    VALUES ("kw1")
+                         , ("kw2")
+                         , ("kw3")
+                   `);
+  // Add bookmarks.
+  let now = Date.now() * 1000;
+  let index = 0;
+  yield db.execute(`INSERT INTO moz_bookmarks (type, fk, parent, position, dateAdded, lastModified, keyword_id, guid)
+                    VALUES (1, (SELECT id FROM moz_places WHERE guid = 'test1_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw1'), "bookmark1___")
+                            /* same uri, different keyword */
+                         , (1, (SELECT id FROM moz_places WHERE guid = 'test1_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw2'), "bookmark2___")
+                           /* different uri, same keyword as 1 */
+                         , (1, (SELECT id FROM moz_places WHERE guid = 'test2_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw1'), "bookmark3___")
+                           /* same uri, same keyword as 1 */
+                         , (1, (SELECT id FROM moz_places WHERE guid = 'test1_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw1'), "bookmark4___")
+                           /* same uri, same keyword as 2 */
+                         , (1, (SELECT id FROM moz_places WHERE guid = 'test2_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw2'), "bookmark5___")
+                           /* different uri, same keyword as 1 */
+                         , (1, (SELECT id FROM moz_places WHERE guid = 'test1_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw3'), "bookmark6___")
+                   `);
+  // Add postData.
+  yield db.execute(`INSERT INTO moz_anno_attributes (name)
+                    VALUES ("bookmarkProperties/POSTData")`);
+  yield db.execute(`INSERT INTO moz_items_annos(anno_attribute_id, item_id, content)
+                    VALUES ((SELECT id FROM moz_anno_attributes where name = "bookmarkProperties/POSTData"),
+                            (SELECT id FROM moz_bookmarks WHERE guid = "bookmark3___"), "postData1")
+                         , ((SELECT id FROM moz_anno_attributes where name = "bookmarkProperties/POSTData"),
+                            (SELECT id FROM moz_bookmarks WHERE guid = "bookmark5___"), "postData2")`);
+  yield db.close();
+});
+
+add_task(function* database_is_valid() {
+  Assert.equal(PlacesUtils.history.databaseStatus,
+               PlacesUtils.history.DATABASE_STATUS_UPGRADED);
+
+  let db = yield PlacesUtils.promiseDBConnection();
+  Assert.equal((yield db.getSchemaVersion()), CURRENT_SCHEMA_VERSION);
+});
+
+add_task(function* test_keywords() {
+  // When 2 urls have the same keyword, if one has postData it will be
+  // preferred.
+  let [ url1, postData1 ] = PlacesUtils.getURLAndPostDataForKeyword("kw1");
+  Assert.equal(url1, "http://test2.com/");
+  Assert.equal(postData1, "postData1");
+  let [ url2, postData2 ] = PlacesUtils.getURLAndPostDataForKeyword("kw2");
+  Assert.equal(url2, "http://test2.com/");
+  Assert.equal(postData2, "postData2");
+  let [ url3, postData3 ] = PlacesUtils.getURLAndPostDataForKeyword("kw3");
+  Assert.equal(url3, "http://test1.com/");
+
+  Assert.equal((yield foreign_count("http://test1.com/")), 5); // 4 bookmark2 + 1 keywords
+  Assert.equal((yield foreign_count("http://test2.com/")), 4); // 2 bookmark2 + 2 keywords
+});

toolkit/components/places/tests/migration/xpcshell.ini

@@ -15,6 +15,7 @@ support-files =
   places_v24.sqlite
   places_v25.sqlite
   places_v26.sqlite
+  places_v27.sqlite
 
 [test_current_from_downgraded.js]
 [test_current_from_v6.js]
@@ -22,3 +23,4 @@ support-files =
 [test_current_from_v19.js]
 [test_current_from_v24.js]
 [test_current_from_v25.js]
+[test_current_from_v26.js]

toolkit/components/places/tests/queries/test_sorting.js

@@ -504,7 +504,7 @@ tests.push({
 
       // if keywords are equal, should fall back to title
       { isBookmark: true,
-        uri: "http://example.com/b2",
+        uri: "http://example.com/b1",
         parentFolder: PlacesUtils.bookmarks.toolbarFolder,
         index: PlacesUtils.bookmarks.DEFAULT_INDEX,
         title: "y8",

toolkit/components/places/tests/unifiedcomplete/test_keyword_search.js

@@ -12,68 +12,60 @@
  * same keyword appear in the list.
  */
 
-add_task(function* test_keyword_searc() {
-  let uri1 = NetUtil.newURI("http://abc/?search=%s");
-  let uri2 = NetUtil.newURI("http://abc/?search=ThisPageIsInHistory");
-  yield PlacesTestUtils.addVisits([
-    { uri: uri1, title: "Generic page title" },
-    { uri: uri2, title: "Generic page title" }
-  ]);
-  addBookmark({ uri: uri1, title: "Keyword title", keyword: "key"});
+// Details for the keyword bookmark
+let keyBase = "http://abc/?search=";
+let keyKey = "key";
 
-  do_print("Plain keyword query");
-  yield check_autocomplete({
-    search: "key term",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=term"), title: "Keyword title", style: ["keyword"] } ]
-  });
+// A second keyword bookmark with the same keyword
+let otherBase = "http://xyz/?foo=";
 
-  do_print("Multi-word keyword query");
-  yield check_autocomplete({
-    search: "key multi word",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=multi+word"), title: "Keyword title", style: ["keyword"] } ]
-  });
+let unescaped = "ユニコード";
+let pageInHistory = "ThisPageIsInHistory";
 
-  do_print("Keyword query with +");
-  yield check_autocomplete({
-    search: "key blocking+",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=blocking%2B"), title: "Keyword title", style: ["keyword"] } ]
-  });
+// Define some shared uris and titles (each page needs its own uri)
+let kURIs = [
+  keyBase + "%s",
+  keyBase + "term",
+  keyBase + "multi+word",
+  keyBase + "blocking%2B",
+  keyBase + unescaped,
+  keyBase + pageInHistory,
+  keyBase,
+  otherBase + "%s",
+  keyBase + "twoKey",
+  otherBase + "twoKey"
+];
+let kTitles = [
+  "Generic page title",
+  "Keyword title",
+];
 
-  do_print("Unescaped term in query");
-  yield check_autocomplete({
-    search: "key ユニコード",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=ユニコード"), title: "Keyword title", style: ["keyword"] } ]
-  });
+// Add the keyword bookmark
+addPageBook(0, 0, 1, [], keyKey);
+// Add in the "fake pages" for keyword searches
+gPages[1] = [1,0];
+gPages[2] = [2,0];
+gPages[3] = [3,0];
+gPages[4] = [4,0];
+// Add a page into history
+addPageBook(5, 0);
+gPages[6] = [6,0];
 
-  do_print("Keyword that happens to match a page");
-  yield check_autocomplete({
-    search: "key ThisPageIsInHistory",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=ThisPageIsInHistory"), title: "Generic page title", style: ["bookmark"] } ]
-  });
-
-  do_print("Keyword without query (without space)");
-  yield check_autocomplete({
-    search: "key",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search="), title: "Keyword title", style: ["keyword"] } ]
-  });
-
-  do_print("Keyword without query (with space)");
-  yield check_autocomplete({
-    search: "key ",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search="), title: "Keyword title", style: ["keyword"] } ]
-  });
-
-  // This adds a second keyword so anything after this will match 2 keywords
-  let uri3 = NetUtil.newURI("http://xyz/?foo=%s");
-  yield PlacesTestUtils.addVisits([ { uri: uri3, title: "Generic page title" } ]);
-  addBookmark({ uri: uri3, title: "Keyword title", keyword: "key", style: ["keyword"] });
-
-  do_print("Two keywords matched");
-  yield check_autocomplete({
-    search: "key twoKey",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=twoKey"), title: "Keyword title", style: ["keyword"] },
-               { uri: NetUtil.newURI("http://xyz/?foo=twoKey"), title: "Keyword title", style: ["keyword"] } ]
-  });
-
-  yield cleanup();
-});
+// Provide for each test: description; search terms; array of gPages indices of
+// pages that should match; optional function to be run before the test
+let gTests = [
+  ["0: Plain keyword query",
+   keyKey + " term", [1]],
+  ["1: Multi-word keyword query",
+   keyKey + " multi word", [2]],
+  ["2: Keyword query with +",
+   keyKey + " blocking+", [3]],
+  ["3: Unescaped term in query",
+   keyKey + " " + unescaped, [4]],
+  ["4: Keyword that happens to match a page",
+   keyKey + " " + pageInHistory, [5]],
+  ["5: Keyword without query (without space)",
+   keyKey, [6]],
+  ["6: Keyword without query (with space)",
+   keyKey + " ", [6]],
+];

toolkit/components/places/tests/unifiedcomplete/test_keyword_search_actions.js

@@ -19,69 +19,57 @@ add_task(function* test_keyword_search() {
     { uri: uri1, title: "Generic page title" },
     { uri: uri2, title: "Generic page title" }
   ]);
-  addBookmark({ uri: uri1, title: "Keyword title", keyword: "key"});
+  yield addBookmark({ uri: uri1, title: "Bookmark title", keyword: "key"});
 
   do_print("Plain keyword query");
   yield check_autocomplete({
     search: "key term",
     searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=term", input: "key term"}), title: "Keyword title", style: [ "action", "keyword" ] } ]
+    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=term", input: "key term"}), title: "Generic page title", style: [ "action", "keyword" ] } ]
   });
 
   do_print("Multi-word keyword query");
   yield check_autocomplete({
     search: "key multi word",
     searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=multi+word", input: "key multi word"}), title: "Keyword title", style: [ "action", "keyword" ] } ]
+    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=multi+word", input: "key multi word"}), title: "Generic page title", style: [ "action", "keyword" ] } ]
   });
 
   do_print("Keyword query with +");
   yield check_autocomplete({
     search: "key blocking+",
     searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=blocking%2B", input: "key blocking+"}), title: "Keyword title", style: [ "action", "keyword" ] } ]
+    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=blocking%2B", input: "key blocking+"}), title: "Generic page title", style: [ "action", "keyword" ] } ]
   });
 
   do_print("Unescaped term in query");
   yield check_autocomplete({
-    search: "key ユニコード",
+    search: "key ¿?¿?¿?¿?¿?",
     searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=ユニコード", input: "key ユニコード"}), title: "Keyword title", style: [ "action", "keyword" ] } ]
+    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=¿?¿?¿?¿?¿?", input: "key ¿?¿?¿?¿?¿?"}), title: "Generic page title", style: [ "action", "keyword" ] } ]
   });
 
   do_print("Keyword that happens to match a page");
   yield check_autocomplete({
     search: "key ThisPageIsInHistory",
     searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=ThisPageIsInHistory", input: "key ThisPageIsInHistory"}), title: "Keyword title", style: [ "action", "keyword" ] } ]
+    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=ThisPageIsInHistory", input: "key ThisPageIsInHistory"}), title: "Generic page title", style: [ "action", "keyword" ] } ]
   });
 
   do_print("Keyword without query (without space)");
   yield check_autocomplete({
     search: "key",
     searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=", input: "key"}), title: "Keyword title", style: [ "action", "keyword" ] } ]
+    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=", input: "key"}), title: "Generic page title", style: [ "action", "keyword" ] } ]
   });
 
   do_print("Keyword without query (with space)");
   yield check_autocomplete({
     search: "key ",
     searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=", input: "key "}), title: "Keyword title", style: [ "action", "keyword" ] } ]
-  });
-
-  // This adds a second keyword so anything after this will match 2 keywords
-  let uri3 = NetUtil.newURI("http://xyz/?foo=%s");
-  yield PlacesTestUtils.addVisits([ { uri: uri3, title: "Generic page title" } ]);
-  addBookmark({ uri: uri3, title: "Keyword title", keyword: "key"});
-
-  do_print("Two keywords matched");
-  yield check_autocomplete({
-    search: "key twoKey",
-    searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=twoKey", input: "key twoKey"}), title: "Keyword title", style: [ "action", "keyword" ] },
-               { uri: makeActionURI("keyword", {url: "http://xyz/?foo=twoKey", input: "key twoKey"}), title: "Keyword title", style: [ "action", "keyword" ] } ]
+    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=", input: "key "}), title: "Generic page title", style: [ "action", "keyword" ] } ]
   });
 
   yield cleanup();
 });
+

toolkit/components/places/tests/unit/test_398914.js

@@ -1,13 +1,3 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim:set ts=2 sw=2 sts=2 et: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-const bmsvc = PlacesUtils.bookmarks;
-const testFolderId = PlacesUtils.bookmarksMenuFolderId;
-
-// main
 function run_test() {
   var testURI = uri("http://foo.com");
 
@@ -16,11 +6,11 @@ function run_test() {
   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");
+  var bm1 = PlacesUtils.bookmarks.insertBookmark(PlacesUtils.bookmarksMenuFolderId, testURI, -1, "blah");
+  PlacesUtils.bookmarks.setKeywordForBookmark(bm1, "foo");
   PlacesUtils.setPostDataForBookmark(bm1, "pdata1");
-  var bm2 = bmsvc.insertBookmark(testFolderId, testURI, -1, "blah");
-  bmsvc.setKeywordForBookmark(bm2, "bar");
+  var bm2 = PlacesUtils.bookmarks.insertBookmark(PlacesUtils.bookmarksMenuFolderId, testURI, -1, "blah");
+  PlacesUtils.bookmarks.setKeywordForBookmark(bm2, "bar");
   PlacesUtils.setPostDataForBookmark(bm2, "pdata2");
 
   // check kw, pd for bookmark 1
@@ -35,116 +25,6 @@ function run_test() {
   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");
-
-  var bm1da = bmsvc.getItemDateAdded(bm1);
-  var bm1lm = bmsvc.getItemLastModified(bm1);
-  LOG("bm1 dateAdded: " + bm1da + ", lastModified: " + bm1lm);
-  var bm2da = bmsvc.getItemDateAdded(bm2);
-  var bm2lm = bmsvc.getItemLastModified(bm2);
-  LOG("bm2 dateAdded: " + bm2da + ", lastModified: " + bm2lm);
-  do_check_true(bm1da <= bm2da);
-  do_check_true(bm1lm <= bm2lm);
-
-  [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");
-
-  var bm1da = bmsvc.getItemDateAdded(bm1);
-  var bm1lm = bmsvc.getItemLastModified(bm1);
-  LOG("bm1 dateAdded: " + bm1da + ", lastModified: " + bm1lm);
-  var bm2da = bmsvc.getItemDateAdded(bm2);
-  var bm2lm = bmsvc.getItemLastModified(bm2);
-  LOG("bm2 dateAdded: " + bm2da + ", lastModified: " + bm2lm);
-  do_check_true(bm1da <= bm2da);
-  // the last modified for bm1 should be at least as big as bm2
-  // but could be equal if the test runs faster than our PRNow()
-  // granularity
-  do_check_true(bm1lm >= bm2lm);
-
-  // we need to ensure that bm1 last modified date is greater
-  // that the modified date of bm2, otherwise in case of a "tie"
-  // bm2 will win, as it has a bigger item id
-  if (bm1lm == bm2lm) 
-    bmsvc.setItemLastModified(bm1, bm2lm + 1000);
-
-  [url, postdata] = PlacesUtils.getURLAndPostDataForKeyword("foo");
-  do_check_eq(testURI.spec, url);
-  do_check_eq(postdata, "pdata1");
-
-  // cleanup
-  bmsvc.removeItem(bm1);
-  bmsvc.removeItem(bm2);
-
-  /*
-  Test that id breaks ties:
-  1. Create two bookmarks with the same URI and keyword, dateAdded and lastModified.
-  2. Confirm that the most recently created one is returned for that keyword.
-  */
-  var testDate = Date.now() * 1000;
-  var bm1 = bmsvc.insertBookmark(testFolderId, testURI, -1, "blah");
-  bmsvc.setKeywordForBookmark(bm1, "foo");
-  PlacesUtils.setPostDataForBookmark(bm1, "pdata1");
-  bmsvc.setItemDateAdded(bm1, testDate);
-  bmsvc.setItemLastModified(bm1, testDate);
-
-  var bm2 = bmsvc.insertBookmark(testFolderId, testURI, -1, "blah");
-  bmsvc.setKeywordForBookmark(bm2, "foo");
-  PlacesUtils.setPostDataForBookmark(bm2, "pdata2");
-  bmsvc.setItemDateAdded(bm2, testDate);
-  bmsvc.setItemLastModified(bm2, testDate);
-
-  var bm1da = bmsvc.getItemDateAdded(bm1, testDate);
-  var bm1lm = bmsvc.getItemLastModified(bm1);
-  LOG("bm1 dateAdded: " + bm1da + ", lastModified: " + bm1lm);
-  var bm2da = bmsvc.getItemDateAdded(bm2);
-  var bm2lm = bmsvc.getItemLastModified(bm2);
-  LOG("bm2 dateAdded: " + bm2da + ", lastModified: " + bm2lm);
-
-  do_check_eq(bm1da, bm2da);
-  do_check_eq(bm1lm, bm2lm);
-
-
-  var ids = bmsvc.getBookmarkIdsForURI(testURI);
-  do_check_eq(ids[0], bm2);
-  do_check_eq(ids[1], bm1);
-
-  [url, postdata] = PlacesUtils.getURLAndPostDataForKeyword("foo");
-  do_check_eq(testURI.spec, url);
-  do_check_eq(postdata, "pdata2");
-
-  // cleanup
-  bmsvc.removeItem(bm1);
-  bmsvc.removeItem(bm2);
+  PlacesUtils.bookmarks.removeItem(bm1);
+  PlacesUtils.bookmarks.removeItem(bm2);
 }

toolkit/components/places/tests/unit/test_421180.js

@@ -1,94 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim:set ts=2 sw=2 sts=2 et: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-// Get bookmarks service
-try {
-  var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
-              getService(Ci.nsINavBookmarksService);
-}
-catch(ex) {
-  do_throw("Could not get bookmarks service\n");
-}
-
-// Get database connection
-try {
-  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
-                getService(Ci.nsINavHistoryService);
-  var mDBConn = histsvc.QueryInterface(Ci.nsPIPlacesDatabase).DBConnection;
-}
-catch(ex) {
-  do_throw("Could not get database connection\n");
-}
-
-add_test(function test_keywordRemovedOnUniqueItemRemoval() {
-  var bookmarkedURI = uri("http://foo.bar");
-  var keyword = "testkeyword";
-
-  // TEST 1
-  // 1. add a bookmark
-  // 2. add a keyword to it
-  // 3. remove bookmark
-  // 4. check that keyword has gone
-  var bookmarkId = bmsvc.insertBookmark(bmsvc.bookmarksMenuFolder,
-                                        bookmarkedURI,
-                                        bmsvc.DEFAULT_INDEX,
-                                        "A bookmark");
-  bmsvc.setKeywordForBookmark(bookmarkId, keyword);
-  // remove bookmark
-  bmsvc.removeItem(bookmarkId);
-
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
-    // Check that keyword has been removed from the database.
-    // The removal is asynchronous.
-    var sql = "SELECT id FROM moz_keywords WHERE keyword = ?1";
-    var stmt = mDBConn.createStatement(sql);
-    stmt.bindByIndex(0, keyword);
-    do_check_false(stmt.executeStep());
-    stmt.finalize();
-
-    run_next_test();
-  });
-});
-
-add_test(function test_keywordNotRemovedOnNonUniqueItemRemoval() {
-  var bookmarkedURI = uri("http://foo.bar");
-  var keyword = "testkeyword";
-
-  // TEST 2
-  // 1. add 2 bookmarks
-  // 2. add the same keyword to them
-  // 3. remove first bookmark
-  // 4. check that keyword is still there
-  var bookmarkId1 = bmsvc.insertBookmark(bmsvc.bookmarksMenuFolder,
-                                        bookmarkedURI,
-                                        bmsvc.DEFAULT_INDEX,
-                                        "A bookmark");
-  bmsvc.setKeywordForBookmark(bookmarkId1, keyword);
-
-  var bookmarkId2 = bmsvc.insertBookmark(bmsvc.toolbarFolder,
-                                        bookmarkedURI,
-                                        bmsvc.DEFAULT_INDEX,
-                                        keyword);
-  bmsvc.setKeywordForBookmark(bookmarkId2, keyword);
-
-  // remove first bookmark
-  bmsvc.removeItem(bookmarkId1);
-
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
-    // check that keyword is still there
-    var sql = "SELECT id FROM moz_keywords WHERE keyword = ?1";
-    var stmt = mDBConn.createStatement(sql);
-    stmt.bindByIndex(0, keyword);
-    do_check_true(stmt.executeStep());
-    stmt.finalize();
-
-    run_next_test();
-  });
-});
-
-function run_test() {
-  run_next_test();
-}

toolkit/components/places/tests/unit/test_async_transactions.js

@@ -739,11 +739,7 @@ add_task(function* test_add_and_remove_bookmarks_with_additional_info() {
    , newValue: ANNO.value },
    { guid: b2_info.guid
    , property: "keyword"
-   , newValue: KEYWORD },
-   { guid: b2_info.guid
-   , isAnnoProperty: true
-   , property: PlacesUtils.POST_DATA_ANNO
-   , newValue: POST_DATA } ];
+   , newValue: KEYWORD } ];
   ensureItemsChanged(...b2_post_creation_changes);
   ensureTags([TAG_1, TAG_2]);
 

toolkit/components/places/tests/unit/test_keywords.js

@@ -0,0 +1,488 @@
+"use strict"
+
+function* check_keyword(aExpectExists, aHref, aKeyword, aPostData = null) {
+  // Check case-insensitivity.
+  aKeyword = aKeyword.toUpperCase();
+
+  let entry = yield PlacesUtils.keywords.fetch(aKeyword);
+  if (aExpectExists) {
+    Assert.ok(!!entry, "A keyword should exist");
+    Assert.equal(entry.url.href, aHref);
+    Assert.equal(entry.postData, aPostData);
+  } else {
+    Assert.ok(!entry || entry.url.href != aHref,
+              "The given keyword entry should not exist");
+  }
+}
+
+/**
+ * Polls the keywords cache waiting for the given keyword entry.
+ */
+function* promiseKeyword(keyword, expectedHref) {
+  let href = null;
+  do {
+    yield new Promise(resolve => do_timeout(100, resolve));
+    let entry = yield PlacesUtils.keywords.fetch(keyword);
+    if (entry)
+      href = entry.url.href; 
+  } while (href != expectedHref);
+}
+
+function* check_no_orphans() {
+  let db = yield PlacesUtils.promiseDBConnection();
+  let rows = yield db.executeCached(
+    `SELECT id FROM moz_keywords k
+     WHERE NOT EXISTS (SELECT 1 FROM moz_places WHERE id = k.place_id)
+    `);
+  Assert.equal(rows.length, 0);
+}
+
+function expectBookmarkNotifications() {
+  let notifications = [];
+  let observer = new Proxy(NavBookmarkObserver, {
+    get(target, name) {
+      if (name == "check") {
+        PlacesUtils.bookmarks.removeObserver(observer);
+        return expectedNotifications =>
+          Assert.deepEqual(notifications, expectedNotifications);
+      }
+
+      if (name.startsWith("onItemChanged")) {
+        return (itemId, property) => {
+          if (property != "keyword")
+            return;
+          let args = Array.from(arguments, arg => {
+            if (arg && arg instanceof Ci.nsIURI)
+              return new URL(arg.spec);
+            if (arg && typeof(arg) == "number" && arg >= Date.now() * 1000)
+              return new Date(parseInt(arg/1000));
+            return arg;
+          });
+          notifications.push({ name: name, arguments: args });
+        }
+      }
+
+      if (name in target)
+        return target[name];
+      return undefined;
+    }
+  });
+  PlacesUtils.bookmarks.addObserver(observer, false);
+  return observer;
+}
+
+add_task(function* test_invalid_input() {
+  Assert.throws(() => PlacesUtils.keywords.fetch(null),
+                /Invalid keyword/);
+  Assert.throws(() => PlacesUtils.keywords.fetch(""),
+                /Invalid keyword/);
+  Assert.throws(() => PlacesUtils.keywords.fetch(5),
+                /Invalid keyword/);
+
+  Assert.throws(() => PlacesUtils.keywords.insert(null),
+                /Input should be a valid object/);
+  Assert.throws(() => PlacesUtils.keywords.insert("test"),
+                /Input should be a valid object/);
+  Assert.throws(() => PlacesUtils.keywords.insert(undefined),
+                /Input should be a valid object/);
+  Assert.throws(() => PlacesUtils.keywords.insert({ }),
+                /Invalid keyword/);
+  Assert.throws(() => PlacesUtils.keywords.insert({ keyword: null }),
+                /Invalid keyword/);
+  Assert.throws(() => PlacesUtils.keywords.insert({ keyword: 5 }),
+                /Invalid keyword/);
+  Assert.throws(() => PlacesUtils.keywords.insert({ keyword: "" }),
+                /Invalid keyword/);
+  Assert.throws(() => PlacesUtils.keywords.insert({ keyword: "test", postData: 5 }),
+                /Invalid POST data/);
+  Assert.throws(() => PlacesUtils.keywords.insert({ keyword: "test", postData: {} }),
+                /Invalid POST data/);
+  Assert.throws(() => PlacesUtils.keywords.insert({ keyword: "test" }),
+                /is not a valid URL/);
+  Assert.throws(() => PlacesUtils.keywords.insert({ keyword: "test", url: 5 }),
+                /is not a valid URL/);
+  Assert.throws(() => PlacesUtils.keywords.insert({ keyword: "test", url: "" }),
+                /is not a valid URL/);
+  Assert.throws(() => PlacesUtils.keywords.insert({ keyword: "test", url: null }),
+                /is not a valid URL/);
+  Assert.throws(() => PlacesUtils.keywords.insert({ keyword: "test", url: "mozilla" }),
+                /is not a valid URL/);
+
+  Assert.throws(() => PlacesUtils.keywords.remove(null),
+                /Invalid keyword/);
+  Assert.throws(() => PlacesUtils.keywords.remove(""),
+                /Invalid keyword/);
+  Assert.throws(() => PlacesUtils.keywords.remove(5),
+                /Invalid keyword/);
+});
+
+add_task(function* test_addKeyword() {
+  yield check_keyword(false, "http://example.com/", "keyword");
+  let fc = yield foreign_count("http://example.com/");
+  let observer = expectBookmarkNotifications();
+
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example.com/" });
+  observer.check([]);
+
+  yield check_keyword(true, "http://example.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 1); // +1 keyword
+
+  // Now remove the keyword.
+  observer = expectBookmarkNotifications();
+  yield PlacesUtils.keywords.remove("keyword");
+  observer.check([]);
+
+  yield check_keyword(false, "http://example.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example.com/")), fc); // -1 keyword
+
+  // Check using URL.
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: new URL("http://example.com/") });
+  yield check_keyword(true, "http://example.com/", "keyword");
+  yield PlacesUtils.keywords.remove("keyword");
+  yield check_keyword(false, "http://example.com/", "keyword");
+
+  yield check_no_orphans();
+});
+
+add_task(function* test_addBookmarkAndKeyword() {
+  yield check_keyword(false, "http://example.com/", "keyword");
+  let fc = yield foreign_count("http://example.com/");
+  let bookmark = yield PlacesUtils.bookmarks.insert({ url: "http://example.com/",
+                                                      type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                      parentGuid: PlacesUtils.bookmarks.unfiledGuid });
+
+  let observer = expectBookmarkNotifications();
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example.com/" });
+
+  observer.check([{ name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark.guid)),
+                                 "keyword", false, "keyword",
+                                 bookmark.lastModified, bookmark.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark.parentGuid)),
+                                 bookmark.guid, bookmark.parentGuid ] } ]);
+
+  yield check_keyword(true, "http://example.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 2); // +1 bookmark +1 keyword
+
+  // Now remove the keyword.
+  observer = expectBookmarkNotifications();
+  yield PlacesUtils.keywords.remove("keyword");
+
+  observer.check([{ name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark.guid)),
+                                 "keyword", false, "",
+                                 bookmark.lastModified, bookmark.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark.parentGuid)),
+                                 bookmark.guid, bookmark.parentGuid ] } ]);
+
+  yield check_keyword(false, "http://example.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 1); // -1 keyword
+
+  // Add again the keyword, then remove the bookmark.
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example.com/" });
+
+  observer = expectBookmarkNotifications();
+  yield PlacesUtils.bookmarks.remove(bookmark.guid);
+  // the notification is synchronous but the removal process is async.
+  // Unfortunately there's nothing explicit we can wait for.
+  while ((yield foreign_count("http://example.com/")));
+  // We don't get any itemChanged notification since the bookmark has been
+  // removed already.
+  observer.check([]);
+
+  yield check_keyword(false, "http://example.com/", "keyword");
+
+  yield check_no_orphans();
+});
+
+add_task(function* test_addKeywordToURIHavingKeyword() {
+  yield check_keyword(false, "http://example.com/", "keyword");
+  let fc = yield foreign_count("http://example.com/");
+
+  let observer = expectBookmarkNotifications();
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example.com/" });
+  observer.check([]);
+
+  yield check_keyword(true, "http://example.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 1); // +1 keyword
+
+  yield PlacesUtils.keywords.insert({ keyword: "keyword2", url: "http://example.com/" });
+
+  yield check_keyword(true, "http://example.com/", "keyword");
+  yield check_keyword(true, "http://example.com/", "keyword2");
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 2); // +1 keyword
+
+  // Now remove the keywords.
+  observer = expectBookmarkNotifications();
+  yield PlacesUtils.keywords.remove("keyword");
+  yield PlacesUtils.keywords.remove("keyword2");
+  observer.check([]);
+
+  yield check_keyword(false, "http://example.com/", "keyword");
+  yield check_keyword(false, "http://example.com/", "keyword2");
+  Assert.equal((yield foreign_count("http://example.com/")), fc); // -1 keyword
+
+  yield check_no_orphans();
+});
+
+add_task(function* test_addBookmarkToURIHavingKeyword() {
+  yield check_keyword(false, "http://example.com/", "keyword");
+  let fc = yield foreign_count("http://example.com/");
+  let observer = expectBookmarkNotifications();
+
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example.com/" });
+  observer.check([]);
+
+  yield check_keyword(true, "http://example.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 1); // +1 keyword
+
+  observer = expectBookmarkNotifications();
+  let bookmark = yield PlacesUtils.bookmarks.insert({ url: "http://example.com/",
+                                                      type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                      parentGuid: PlacesUtils.bookmarks.unfiledGuid });
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 2); // +1 bookmark
+  observer.check([]);
+
+  observer = expectBookmarkNotifications();
+  yield PlacesUtils.bookmarks.remove(bookmark.guid);
+  // the notification is synchronous but the removal process is async.
+  // Unfortunately there's nothing explicit we can wait for.
+  while ((yield foreign_count("http://example.com/")));
+  // We don't get any itemChanged notification since the bookmark has been
+  // removed already.
+  observer.check([]);
+
+  yield check_keyword(false, "http://example.com/", "keyword");
+
+  yield check_no_orphans();
+});
+
+add_task(function* test_sameKeywordDifferentURL() {
+  let fc1 = yield foreign_count("http://example1.com/");
+  let bookmark1 = yield PlacesUtils.bookmarks.insert({ url: "http://example1.com/",
+                                                       type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                       parentGuid: PlacesUtils.bookmarks.unfiledGuid });
+  let fc2 = yield foreign_count("http://example2.com/");
+  let bookmark2 = yield PlacesUtils.bookmarks.insert({ url: "http://example2.com/",
+                                                       type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                       parentGuid: PlacesUtils.bookmarks.unfiledGuid });
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example1.com/" });
+
+  yield check_keyword(true, "http://example1.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example1.com/")), fc1 + 2); // +1 bookmark +1 keyword
+  yield check_keyword(false, "http://example2.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example2.com/")), fc2 + 1); // +1 bookmark
+
+  // Assign the same keyword to another url.
+  let observer = expectBookmarkNotifications();
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example2.com/" });
+
+  observer.check([{ name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark1.guid)),
+                                 "keyword", false, "",
+                                 bookmark1.lastModified, bookmark1.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark1.parentGuid)),
+                                 bookmark1.guid, bookmark1.parentGuid ] },
+                  { name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark2.guid)),
+                                 "keyword", false, "keyword",
+                                 bookmark2.lastModified, bookmark2.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark2.parentGuid)),
+                                 bookmark2.guid, bookmark2.parentGuid ] } ]);
+
+  yield check_keyword(false, "http://example1.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example1.com/")), fc1 + 1); // -1 keyword
+  yield check_keyword(true, "http://example2.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example2.com/")), fc2 + 2); // +1 keyword
+
+  // Now remove the keyword.
+  observer = expectBookmarkNotifications();
+  yield PlacesUtils.keywords.remove("keyword");
+  observer.check([{ name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark2.guid)),
+                                 "keyword", false, "",
+                                 bookmark2.lastModified, bookmark2.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark2.parentGuid)),
+                                 bookmark2.guid, bookmark2.parentGuid ] } ]);
+
+  yield check_keyword(false, "http://example1.com/", "keyword");
+  yield check_keyword(false, "http://example2.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example1.com/")), fc1 + 1);
+  Assert.equal((yield foreign_count("http://example2.com/")), fc2 + 1); // -1 keyword
+
+  yield PlacesUtils.bookmarks.remove(bookmark1);
+  yield PlacesUtils.bookmarks.remove(bookmark2);
+  Assert.equal((yield foreign_count("http://example1.com/")), fc1); // -1 bookmark
+  while ((yield foreign_count("http://example2.com/"))); // -1 keyword
+
+  yield check_no_orphans();
+});
+
+add_task(function* test_sameURIDifferentKeyword() {
+  let fc = yield foreign_count("http://example.com/");
+
+  let observer = expectBookmarkNotifications();
+  let bookmark = yield PlacesUtils.bookmarks.insert({ url: "http://example.com/",
+                                                      type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                      parentGuid: PlacesUtils.bookmarks.unfiledGuid });
+  yield PlacesUtils.keywords.insert({keyword: "keyword", url: "http://example.com/" });
+
+  yield check_keyword(true, "http://example.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 2); // +1 bookmark +1 keyword
+
+  observer.check([{ name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark.guid)),
+                                 "keyword", false, "keyword",
+                                 bookmark.lastModified, bookmark.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark.parentGuid)),
+                                 bookmark.guid, bookmark.parentGuid ] } ]);
+
+  observer = expectBookmarkNotifications();
+  yield PlacesUtils.keywords.insert({ keyword: "keyword2", url: "http://example.com/" });
+  yield check_keyword(true, "http://example.com/", "keyword");
+  yield check_keyword(true, "http://example.com/", "keyword2");
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 3); // +1 keyword
+  observer.check([{ name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark.guid)),
+                                 "keyword", false, "keyword2",
+                                 bookmark.lastModified, bookmark.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark.parentGuid)),
+                                 bookmark.guid, bookmark.parentGuid ] } ]);
+
+  // Add a third keyword.
+  yield PlacesUtils.keywords.insert({ keyword: "keyword3", url: "http://example.com/" });
+  yield check_keyword(true, "http://example.com/", "keyword");
+  yield check_keyword(true, "http://example.com/", "keyword2");
+  yield check_keyword(true, "http://example.com/", "keyword3");
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 4); // +1 keyword
+
+  // Remove one of the keywords.
+  observer = expectBookmarkNotifications();
+  yield PlacesUtils.keywords.remove("keyword");
+  yield check_keyword(false, "http://example.com/", "keyword");
+  yield check_keyword(true, "http://example.com/", "keyword2");
+  yield check_keyword(true, "http://example.com/", "keyword3");
+  observer.check([{ name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark.guid)),
+                                 "keyword", false, "",
+                                 bookmark.lastModified, bookmark.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark.parentGuid)),
+                                 bookmark.guid, bookmark.parentGuid ] } ]);
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 3); // -1 keyword
+
+  // Now remove the bookmark.
+  yield PlacesUtils.bookmarks.remove(bookmark);
+  while ((yield foreign_count("http://example.com/")));
+  yield check_keyword(false, "http://example.com/", "keyword");
+  yield check_keyword(false, "http://example.com/", "keyword2");
+  yield check_keyword(false, "http://example.com/", "keyword3");
+
+  check_no_orphans();
+});
+
+add_task(function* test_deleteKeywordMultipleBookmarks() {
+  let fc = yield foreign_count("http://example.com/");
+
+  let observer = expectBookmarkNotifications();
+  let bookmark1 = yield PlacesUtils.bookmarks.insert({ url: "http://example.com/",
+                                                       type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                       parentGuid: PlacesUtils.bookmarks.unfiledGuid });
+  let bookmark2 = yield PlacesUtils.bookmarks.insert({ url: "http://example.com/",
+                                                       type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                       parentGuid: PlacesUtils.bookmarks.unfiledGuid });
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example.com/" });
+
+  yield check_keyword(true, "http://example.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 3); // +2 bookmark +1 keyword
+  observer.check([{ name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark2.guid)),
+                                 "keyword", false, "keyword",
+                                 bookmark2.lastModified, bookmark2.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark2.parentGuid)),
+                                 bookmark2.guid, bookmark2.parentGuid ] },
+                  { name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark1.guid)),
+                                 "keyword", false, "keyword",
+                                 bookmark1.lastModified, bookmark1.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark1.parentGuid)),
+                                 bookmark1.guid, bookmark1.parentGuid ] } ]);
+
+  observer = expectBookmarkNotifications();
+  yield PlacesUtils.keywords.remove("keyword");
+  yield check_keyword(false, "http://example.com/", "keyword");
+  Assert.equal((yield foreign_count("http://example.com/")), fc + 2); // -1 keyword
+  observer.check([{ name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark2.guid)),
+                                 "keyword", false, "",
+                                 bookmark2.lastModified, bookmark2.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark2.parentGuid)),
+                                 bookmark2.guid, bookmark2.parentGuid ] },
+                  { name: "onItemChanged",
+                    arguments: [ (yield PlacesUtils.promiseItemId(bookmark1.guid)),
+                                 "keyword", false, "",
+                                 bookmark1.lastModified, bookmark1.type,
+                                 (yield PlacesUtils.promiseItemId(bookmark1.parentGuid)),
+                                 bookmark1.guid, bookmark1.parentGuid ] } ]);
+
+  // Now remove the bookmarks.
+  yield PlacesUtils.bookmarks.remove(bookmark1);
+  yield PlacesUtils.bookmarks.remove(bookmark2);
+  Assert.equal((yield foreign_count("http://example.com/")), fc); // -2 bookmarks
+
+  check_no_orphans();
+});
+
+add_task(function* test_multipleKeywordsSamePostData() {
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example.com/", postData: "postData1" });
+  yield check_keyword(true, "http://example.com/", "keyword", "postData1");
+  // Add another keyword with same postData, should fail.
+  yield Assert.rejects(PlacesUtils.keywords.insert({ keyword: "keyword2", url: "http://example.com/", postData: "postData1" }),
+                       /constraint failed/);
+  yield check_keyword(false, "http://example.com/", "keyword2", "postData1");
+
+  yield PlacesUtils.keywords.remove("keyword");
+
+  check_no_orphans();
+});
+
+add_task(function* test_oldPostDataAPI() {
+  let bookmark = yield PlacesUtils.bookmarks.insert({ url: "http://example.com/",
+                                                      type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                      parentGuid: PlacesUtils.bookmarks.unfiledGuid });
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example.com/" });
+  let itemId = yield PlacesUtils.promiseItemId(bookmark.guid);
+  yield PlacesUtils.setPostDataForBookmark(itemId, "postData");
+  yield check_keyword(true, "http://example.com/", "keyword", "postData");
+  Assert.equal(PlacesUtils.getPostDataForBookmark(itemId), "postData");
+
+  yield PlacesUtils.keywords.remove("keyword");
+  yield PlacesUtils.bookmarks.remove(bookmark);
+
+  check_no_orphans();
+});
+
+add_task(function* test_oldKeywordsAPI() {
+  let bookmark = yield PlacesUtils.bookmarks.insert({ url: "http://example.com/",
+                                                    type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                    parentGuid: PlacesUtils.bookmarks.unfiledGuid });
+  yield check_keyword(false, "http://example.com/", "keyword");
+  let itemId = yield PlacesUtils.promiseItemId(bookmark.guid);
+
+  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword");
+  yield promiseKeyword("keyword", "http://example.com/");
+
+  // Remove the keyword.
+  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "");
+  yield promiseKeyword("keyword", null);
+
+  yield PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example.com" });
+  Assert.equal(PlacesUtils.bookmarks.getKeywordForBookmark(itemId), "keyword");
+  Assert.equal(PlacesUtils.bookmarks.getURIForKeyword("keyword").spec, "http://example.com/");
+  yield PlacesUtils.bookmarks.remove(bookmark);
+
+  check_no_orphans();
+});
+
+function run_test() {
+  run_next_test();
+}

toolkit/components/places/tests/unit/test_placesTxn.js

@@ -717,44 +717,25 @@ add_test(function test_sort_folder_by_name() {
 });
 
 add_test(function test_edit_postData() {
-  function* promiseKeyword(keyword, href, postData) {
-    while (true) {
-      let entry = yield PlacesUtils.keywords.fetch(keyword);
-      if (entry && entry.url.href == href && entry.postData == postData) {
-        break;
-      }
+  const POST_DATA_ANNO = "bookmarkProperties/POSTData";
+  let postData = "post-test_edit_postData";
+  let testURI = NetUtil.newURI("http://test_edit_postData.com");
+  let testBkmId = bmsvc.insertBookmark(root, testURI, bmsvc.DEFAULT_INDEX, "Test edit Post Data");
+  PlacesUtils.bookmarks.setKeywordForBookmark(testBkmId, "kw");
+  let txn = new PlacesEditBookmarkPostDataTransaction(testBkmId, postData);
 
-      yield new Promise(resolve => do_timeout(100, resolve));
-    }
-  }
+  txn.doTransaction();
+  let [url, post_data] = PlacesUtils.getURLAndPostDataForKeyword("kw");
+  Assert.equal(url, testURI.spec);
+  Assert.equal(postData, post_data);
 
-  Task.spawn(function* () {
-    let postData = "post-test_edit_postData";
-    let testURI = NetUtil.newURI("http://test_edit_postData.com");
+  txn.undoTransaction();
+  [url, post_data] = PlacesUtils.getURLAndPostDataForKeyword("kw");
+  Assert.equal(url, testURI.spec);
+  Assert.equal(null, post_data);
 
-    let testBkm = yield PlacesUtils.bookmarks.insert({
-      parentGuid: PlacesUtils.bookmarks.menuGuid,
-      url: "http://test_edit_postData.com",
-      title: "Test edit Post Data"
-    });
 
-    yield PlacesUtils.keywords.insert({
-      keyword: "kw",
-      url: "http://test_edit_postData.com"
-    });
-
-    let testBkmId = yield PlacesUtils.promiseItemId(testBkm.guid);
-    let txn = new PlacesEditBookmarkPostDataTransaction(testBkmId, postData);
-
-    txn.doTransaction();
-    yield promiseKeyword("kw", testURI.spec, postData);
-
-    txn.undoTransaction();
-    entry = yield PlacesUtils.keywords.fetch("kw");
-    Assert.equal(entry.url.href, testURI.spec);
-    // We don't allow anymore to set a null post data.
-    //Assert.equal(null, post_data);
-  }).then(run_next_test);
+  run_next_test();
 });
 
 add_test(function test_tagURI_untagURI() {
@@ -966,4 +947,4 @@ add_test(function test_create_folder_with_child_itemTxn() {
 
   run_next_test();
 });
-
+i

toolkit/components/places/tests/unit/test_preventive_maintenance.js

@@ -544,53 +544,6 @@ tests.push({
 
 //------------------------------------------------------------------------------
 
-tests.push({
-  name: "D.5",
-  desc: "Fix wrong keywords",
-
-  _validKeywordItemId: null,
-  _invalidKeywordItemId: null,
-  _validKeywordId: 1,
-  _invalidKeywordId: 8888,
-  _placeId: null,
-
-  setup: function() {
-    // Insert a keyword
-    let stmt = mDBConn.createStatement("INSERT INTO moz_keywords (id, keyword) VALUES(:id, :keyword)");
-    stmt.params["id"] = this._validKeywordId;
-    stmt.params["keyword"] = "used";
-    stmt.execute();
-    stmt.finalize();
-    // Add a place to ensure place_id = 1 is valid
-    this._placeId = addPlace();
-    // Add a bookmark using the keyword
-    this._validKeywordItemId = addBookmark(this._placeId, bs.TYPE_BOOKMARK, bs.unfiledBookmarksFolder, this._validKeywordId);
-    // Add a bookmark using a nonexistent keyword
-    this._invalidKeywordItemId = addBookmark(this._placeId, bs.TYPE_BOOKMARK, bs.unfiledBookmarksFolder, this._invalidKeywordId);
-  },
-
-  check: function() {
-    // Check that item with valid keyword is there
-    let stmt = mDBConn.createStatement("SELECT id FROM moz_bookmarks WHERE id = :item_id AND keyword_id = :keyword");
-    stmt.params["item_id"] = this._validKeywordItemId;
-    stmt.params["keyword"] = this._validKeywordId;
-    do_check_true(stmt.executeStep());
-    stmt.reset();
-    // Check that item with invalid keyword has been corrected
-    stmt.params["item_id"] = this._invalidKeywordItemId;
-    stmt.params["keyword"] = this._invalidKeywordId;
-    do_check_false(stmt.executeStep());
-    stmt.finalize();
-    // Check that item with invalid keyword has not been removed
-    stmt = mDBConn.createStatement("SELECT id FROM moz_bookmarks WHERE id = :item_id");
-    stmt.params["item_id"] = this._invalidKeywordItemId;
-    do_check_true(stmt.executeStep());
-    stmt.finalize();
-  }
-});
-
-//------------------------------------------------------------------------------
-
 tests.push({
   name: "D.6",
   desc: "Fix wrong item types | bookmarks",
@@ -1053,27 +1006,17 @@ tests.push({
 
   setup: function() {
     // Insert 2 keywords
-    let stmt = mDBConn.createStatement("INSERT INTO moz_keywords (id, keyword) VALUES(:id, :keyword)");
+    let stmt = mDBConn.createStatement("INSERT INTO moz_keywords (id, keyword, place_id) VALUES(:id, :keyword, :place_id)");
     stmt.params["id"] = 1;
-    stmt.params["keyword"] = "used";
-    stmt.execute();
-    stmt.reset();
-    stmt.params["id"] = 2;
     stmt.params["keyword"] = "unused";
+    stmt.params["place_id"] = 100;
     stmt.execute();
     stmt.finalize();
-    // Add a place to ensure place_id = 1 is valid
-    this._placeId = addPlace();
-    // Insert a bookmark using the "used" keyword
-    this._bookmarkId = addBookmark(this._placeId, bs.TYPE_BOOKMARK, bs.unfiledBookmarksFolder, 1);
   },
 
   check: function() {
     // Check that "used" keyword is still there
     let stmt = mDBConn.createStatement("SELECT id FROM moz_keywords WHERE keyword = :keyword");
-    stmt.params["keyword"] = "used";
-    do_check_true(stmt.executeStep());
-    stmt.reset();
     // Check that "unused" keyword has gone
     stmt.params["keyword"] = "unused";
     do_check_false(stmt.executeStep());

toolkit/components/places/tests/unit/test_telemetry.js

@@ -32,7 +32,7 @@ function run_test()
 add_task(function test_execute()
 {
   // Put some trash in the database.
-  const URI = NetUtil.newURI("http://moz.org/");
+  let uri = NetUtil.newURI("http://moz.org/");
 
   let folderId = PlacesUtils.bookmarks.createFolder(PlacesUtils.unfiledBookmarksFolderId,
                                                     "moz test",
@@ -64,7 +64,7 @@ add_task(function test_execute()
   // Test expiration probes.
   for (let i = 0; i < 2; i++) {
     yield PlacesTestUtils.addVisits({
-      uri: uri("http://" +  i + ".moz.org/"),
+      uri: NetUtil.newURI("http://" +  i + ".moz.org/"),
       visitDate: Date.now() // [sic]
     });
   }

toolkit/content/aboutSupport.js

@@ -692,24 +692,14 @@ function populateActionBox() {
   }
 }
 
-// Restart the browser
-function restart(safeMode) {
-  // Notify all windows that an application quit has been requested.
+// Prompt user to restart the browser in safe mode
+function safeModeRestart() {
   let cancelQuit = Cc["@mozilla.org/supports-PRBool;1"]
                      .createInstance(Ci.nsISupportsPRBool);
   Services.obs.notifyObservers(cancelQuit, "quit-application-requested", "restart");
 
-  // Something aborted the quit process.
-  if (cancelQuit.data) {
-    return;
-  }
-
-  let flags = Ci.nsIAppStartup.eAttemptQuit;
-
-  if (safeMode) {
-    Services.startup.restartInSafeMode(flags);
-  } else {
-    Services.startup.quit(flags | Ci.nsIAppStartup.eRestart);
+  if (!cancelQuit.data) {
+    Services.startup.restartInSafeMode(Ci.nsIAppStartup.eAttemptQuit);
   }
 }
 
@@ -740,12 +730,9 @@ function setupEventListeners(){
       Services.obs.notifyObservers(null, "restart-in-safe-mode", "");
     }
     else {
-      restart(true);
+      safeModeRestart();
     }
   });
-  $("restart-button").addEventListener("click", function (event) {
-    restart(false);
-  });
   $("verify-place-integrity-button").addEventListener("click", function(event) {
     PlacesDBUtils.checkAndFixDatabase(function(aLog) {
       let msg = aLog.join("\n");

toolkit/content/aboutSupport.xhtml

@@ -42,12 +42,6 @@
           &aboutSupport.restartInSafeMode.label;
         </button>
       </div>
-      <div id="restart-box">
-        <h3>&aboutSupport.restartTitle;</h3>
-        <button id="restart-button">
-          &aboutSupport.restartNormal.label;
-        </button>
-      </div>
 
     </div>
     <h1>

toolkit/locales/en-US/chrome/global/aboutSupport.dtd

@@ -108,6 +108,3 @@ variant of aboutSupport.showDir.label. -->
 
 <!ENTITY aboutSupport.safeModeTitle "Try Safe Mode">
 <!ENTITY aboutSupport.restartInSafeMode.label "Restart in Safe Mode…">
-
-<!ENTITY aboutSupport.restartTitle "Try Restart">
-<!ENTITY aboutSupport.restartNormal.label "Restart normally…">