Bug 1506461 - Enable inline suggestions for bookmark tags autocomplete. r=Standard8

Differential Revision: https://phabricator.services.mozilla.com/D11773

--HG--
extra : moz-landing-system : lando

browser/base/content/browser-places.js

@@ -286,7 +286,10 @@ var StarUI = {
       return;
     this._batchBlockingDeferred = PromiseUtils.defer();
     PlacesTransactions.batch(async () => {
+      // First await for the batch to be concluded.
       await this._batchBlockingDeferred.promise;
+      // And then for any pending promises added in the meanwhile.
+      await Promise.all(gEditItemOverlay.transactionPromises);
     });
     this._batching = true;
   },

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

@@ -87,7 +87,8 @@ async function test_bookmarks_popup({isNewBookmark, popupShowFn, popupEditFn,
       let onItemRemovedPromise = Promise.resolve();
       if (isBookmarkRemoved) {
         onItemRemovedPromise = PlacesTestUtils.waitForNotification("onItemRemoved",
-          (id, parentId, index, type, itemUrl) => TEST_URL == itemUrl.spec);
+          (id, parentId, index, type, uri, guid, parentGuid) =>
+            parentGuid == PlacesUtils.bookmarks.unfiledGuid && TEST_URL == uri.spec);
       }
 
       let hiddenPromise = promisePopupHidden(bookmarkPanel);
@@ -137,11 +138,11 @@ add_task(async function panel_shown_once_for_slow_doubleclick_on_new_bookmark_st
               "browser-places.js for this.");
 
   /*
-  yield test_bookmarks_popup({
+  await test_bookmarks_popup({
     isNewBookmark: true,
     *popupShowFn() {
       EventUtils.synthesizeMouse(bookmarkStar, 10, 10, window);
-      yield new Promise(resolve => setTimeout(resolve, 300));
+      await new Promise(resolve => setTimeout(resolve, 300));
       EventUtils.synthesizeMouse(bookmarkStar, 10, 10, window);
     },
     shouldAutoClose: true,
@@ -480,7 +481,7 @@ add_task(async function enter_during_autocomplete_should_prevent_autoclose() {
     popupHideFn() {
       EventUtils.synthesizeKey("KEY_Escape", {}, window);
     },
-    isBookmarkRemoved: false,
+    isBookmarkRemoved: true,
   });
 });
 
@@ -511,17 +512,20 @@ add_task(async function escape_during_autocomplete_should_prevent_autoclose() {
       // Select first candidate.
       EventUtils.synthesizeKey("KEY_ArrowDown", {}, window);
 
-      // Type Enter key to choose the item.
+      // Type Escape key to close autocomplete.
       EventUtils.synthesizeKey("KEY_Escape", {}, window);
 
-      Assert.equal(tagsField.value, "Abc",
-        "Autocomplete should've inserted the selected item and shouldn't clear it");
+      // The text reverts to what was typed.
+      // Note, it's important that this is different from the previously
+      // inserted tag, since it will test an untag/tag undo condition.
+      Assert.equal(tagsField.value, "a",
+        "Autocomplete should revert to what was typed");
     },
     shouldAutoClose: false,
     popupHideFn() {
       EventUtils.synthesizeKey("KEY_Escape", {}, window);
     },
-    isBookmarkRemoved: false,
+    isBookmarkRemoved: true,
   });
 });
 

browser/components/places/content/editBookmark.js

@@ -7,6 +7,9 @@ ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 const MAX_FOLDER_ITEM_IN_MENU_LIST = 5;
 
 var gEditItemOverlay = {
+  // Array of PlacesTransactions accumulated by internal changes. It can be used
+  // to wait for completion.
+  transactionPromises: null,
   _observersAdded: false,
   _staticFoldersListBuilt: false,
 
@@ -196,6 +199,8 @@ var gEditItemOverlay = {
     if (this.initialized)
       this.uninitPanel(false);
 
+    this.transactionPromises = [];
+
     let { parentGuid, isItem, isURI,
           isBookmark, bulkTagging, uris,
           visibleRows, focusedElement,
@@ -464,6 +469,7 @@ var gEditItemOverlay = {
 
     this._setPaneInfo(null);
     this._firstEditedField = "";
+    this.transactionPromises = [];
   },
 
   get selectedFolderGuid() {
@@ -511,27 +517,38 @@ var gEditItemOverlay = {
   // Adds and removes tags for one or more uris.
   _setTagsFromTagsInputField(aCurrentTags, aURIs) {
     let { removedTags, newTags } = this._getTagsChanges(aCurrentTags);
-    if (removedTags.length + newTags.length == 0)
+    if (removedTags.length + newTags.length == 0) {
       return false;
+    }
 
-    let setTags = async function() {
+    let setTags = async () => {
+      let promises = [];
       if (removedTags.length > 0) {
-        await PlacesTransactions.Untag({ urls: aURIs, tags: removedTags })
-                                .transact();
+        let promise = PlacesTransactions.Untag({ urls: aURIs, tags: removedTags })
+                                        .transact().catch(Cu.reportError);
+        this.transactionPromises.push(promise);
+        promises.push(promise);
       }
       if (newTags.length > 0) {
-        await PlacesTransactions.Tag({ urls: aURIs, tags: newTags })
-                                .transact();
+        let promise = PlacesTransactions.Tag({ urls: aURIs, tags: newTags })
+                                        .transact().catch(Cu.reportError);
+        this.transactionPromises.push(promise);
+        promises.push(promise);
+      }
+      // Don't use Promise.all because we want these to be executed in order.
+      for (let promise of promises) {
+        await promise;
       }
     };
 
     // Only in the library info-pane it's safe (and necessary) to batch these.
     // TODO bug 1093030: cleanup this mess when the bookmarksProperties dialog
     // and star UI code don't "run a batch in the background".
-    if (window.document.documentElement.id == "places")
-      PlacesTransactions.batch(setTags).catch(Cu.reportError);
-    else
-      setTags().catch(Cu.reportError);
+    if (window.document.documentElement.id == "places") {
+      PlacesTransactions.batch(setTags);
+    } else {
+      setTags();
+    }
     return true;
   },
 
@@ -597,13 +614,18 @@ var gEditItemOverlay = {
       if (title == oldTag) {
         this._paneInfo.title = tag;
       }
-      await PlacesTransactions.RenameTag({ oldTag, tag }).transact();
+      let promise = PlacesTransactions.RenameTag({ oldTag, tag }).transact();
+      this.transactionPromises.push(promise.catch(Cu.reportError));
+      await promise;
       return;
     }
 
     this._mayUpdateFirstEditField("namePicker");
-    await PlacesTransactions.EditTitle({ guid: this._paneInfo.itemGuid,
-                                         title: this._namePicker.value }).transact();
+    let promise = PlacesTransactions.EditTitle({ guid: this._paneInfo.itemGuid,
+                                                 title: this._namePicker.value })
+                                    .transact();
+    this.transactionPromises.push(promise.catch(Cu.reportError));
+    await promise;
   },
 
   onLocationFieldChange() {
@@ -622,8 +644,8 @@ var gEditItemOverlay = {
       return;
 
     let guid = this._paneInfo.itemGuid;
-    PlacesTransactions.EditUrl({ guid, url: newURI })
-                      .transact().catch(Cu.reportError);
+    this.transactionPromises.push(PlacesTransactions.EditUrl({ guid, url: newURI })
+                                                    .transact().catch(Cu.reportError));
   },
 
   onKeywordFieldChange() {
@@ -634,8 +656,8 @@ var gEditItemOverlay = {
     let keyword = this._keyword = this._keywordField.value;
     let postData = this._paneInfo.postData;
     let guid = this._paneInfo.itemGuid;
-    PlacesTransactions.EditKeyword({ guid, keyword, postData, oldKeyword })
-                      .transact().catch(Cu.reportError);
+    this.transactionPromises.push(PlacesTransactions.EditKeyword({ guid, keyword, postData, oldKeyword })
+                                                    .transact().catch(Cu.reportError));
   },
 
   toggleFolderTreeVisibility() {
@@ -717,10 +739,12 @@ var gEditItemOverlay = {
     let containerGuid = this._folderMenuList.selectedItem.folderGuid;
     if (this._paneInfo.parentGuid != containerGuid &&
         this._paneInfo.itemGuid != containerGuid) {
-      await PlacesTransactions.Move({
+      let promise = PlacesTransactions.Move({
         guid: this._paneInfo.itemGuid,
         newParentGuid: containerGuid,
       }).transact();
+      this.transactionPromises.push(promise.catch(Cu.reportError));
+      await promise;
 
       // Auto-show the bookmarks toolbar when adding / moving an item there.
       if (containerGuid == PlacesUtils.bookmarks.toolbarGuid) {
@@ -849,11 +873,13 @@ var gEditItemOverlay = {
 
     // XXXmano: add a separate "New Folder" string at some point...
     let title = this._element("newFolderButton").label;
-    let guid = await PlacesTransactions.NewFolder({
+    let promise = PlacesTransactions.NewFolder({
       parentGuid: ip.guid,
       title,
       index: await ip.getIndex(),
-    }).transact().catch(Cu.reportError);
+    }).transact();
+    this.transactionPromises.push(promise.catch(Cu.reportError));
+    let guid = await promise;
 
     this._folderTree.focus();
     this._folderTree.selectItems([ip.guid]);

browser/components/places/content/editBookmarkPanel.inc.xul

@@ -94,6 +94,7 @@
                  autocompletesearch="places-tag-autocomplete"
                  autocompletepopup="PopupAutoComplete"
                  completedefaultindex="true"
+                 completeselectedindex="true"
                  tabscrolling="true"
                  placeholder="&editBookmarkOverlay.tagsEmptyDesc.label;"
                  onchange="gEditItemOverlay.onTagsFieldChange();"/>

toolkit/components/places/nsTaggingService.js

@@ -446,6 +446,7 @@ TagAutoCompleteSearch.prototype = {
     // regardless having matches.
     let result = Cc["@mozilla.org/autocomplete/simple-result;1"]
                    .createInstance(Ci.nsIAutoCompleteSimpleResult);
+    result.setDefaultIndex(0);
     result.setSearchString(searchString);
 
     let count = 0;