Bug 390739 - Redo menu item is greyed out in Bookmarks Organizer (was: doesn't work when deleting items and undoing in Bookmarks Manager). r=dietrich.

browser/base/content/browser-places.js

@@ -195,7 +195,7 @@ var PlacesCommandHook = {
       var descAnno = { name: DESCRIPTION_ANNO, value: description };
       var txn = PlacesUtils.ptm.createItem(uri, parent, -1,
                                            title, null, [descAnno]);
-      PlacesUtils.ptm.commitTransaction(txn);
+      PlacesUtils.ptm.doTransaction(txn);
       if (aShowEditUI)
         itemId = PlacesUtils.getMostRecentBookmarkForURI(uri);
     }

browser/base/content/nsContextMenu.js

@@ -1160,7 +1160,7 @@ nsContextMenu.prototype = {
       var descAnno = { name: DESCRIPTION_ANNO, value: description };
       var txn = PlacesUtils.ptm.createItem(uri, PlacesUtils.bookmarksRootId, -1,
                                            title, null, [descAnno]);
-      PlacesUtils.ptm.commitTransaction(txn);
+      PlacesUtils.ptm.doTransaction(txn);
       itemId = PlacesUtils.getMostRecentBookmarkForURI(uri);
     }
 

browser/components/places/Makefile.in

@@ -50,3 +50,5 @@ ifdef ENABLE_TESTS
 endif
 
 include $(topsrcdir)/config/rules.mk
+
+XPIDL_FLAGS += -I$(topsrcdir)/browser/components/

browser/components/places/content/bookmarkProperties.js

@@ -345,8 +345,6 @@ var BookmarkPropertiesPanel = {
    * dialog to initialize the state of the panel.
    */
   onDialogLoad: function BPP_onDialogLoad() {
-    this._tm = window.opener.PlacesUtils.tm;
-
     this._determineItemInfo();
     this._populateProperties();
     this._forceHideRows();
@@ -883,7 +881,7 @@ var BookmarkPropertiesPanel = {
       window.arguments[0].performed = true;
       var aggregate =
         PlacesUtils.ptm.aggregateTransactions(this._getDialogTitle(), transactions);
-      this._tm.doTransaction(aggregate);
+      PlacesUtils.ptm.doTransaction(aggregate);
     }
   },
 
@@ -1015,7 +1013,7 @@ var BookmarkPropertiesPanel = {
     // perfrom our transaction do via the transaction manager passed by the
     // opener so it can be undone.
     window.arguments[0].performed = true;
-    this._tm.doTransaction(createTxn);
+    PlacesUtils.ptm.doTransaction(createTxn);
   },
 
   onNamePickerInput: function BPP_onNamePickerInput() {

browser/components/places/content/controller.js

@@ -93,9 +93,9 @@ PlacesController.prototype = {
   isCommandEnabled: function PC_isCommandEnabled(aCommand) {
     switch (aCommand) {
     case "cmd_undo":
-      return PlacesUtils.tm.numberOfUndoItems > 0;
+      return PlacesUtils.ptm.numberOfUndoItems > 0;
     case "cmd_redo":
-      return PlacesUtils.tm.numberOfRedoItems > 0;
+      return PlacesUtils.ptm.numberOfRedoItems > 0;
     case "cmd_cut":
     case "cmd_delete":
       return this._hasRemovableSelection(false);
@@ -197,10 +197,10 @@ PlacesController.prototype = {
   doCommand: function PC_doCommand(aCommand) {
     switch (aCommand) {
     case "cmd_undo":
-      PlacesUtils.tm.undoTransaction();
+      PlacesUtils.ptm.undoTransaction();
       break;
     case "cmd_redo":
-      PlacesUtils.tm.redoTransaction();
+      PlacesUtils.ptm.redoTransaction();
       break;
     case "cmd_cut":
       this.cut();
@@ -802,7 +802,7 @@ PlacesController.prototype = {
     if (!ip)
       throw Cr.NS_ERROR_NOT_AVAILABLE;
     var txn = PlacesUtils.ptm.createSeparator(ip.itemId, ip.index);
-    PlacesUtils.ptm.commitTransaction(txn);
+    PlacesUtils.ptm.doTransaction(txn);
   },
 
   /**
@@ -811,7 +811,7 @@ PlacesController.prototype = {
   moveSelectedBookmarks: function PC_moveBookmarks() {
     window.openDialog("chrome://browser/content/places/moveBookmarks.xul",
                       "", "chrome, modal",
-                      this._view.getSelectionNodes(), PlacesUtils.tm);
+                      this._view.getSelectionNodes());
   },
 
   /**
@@ -821,7 +821,7 @@ PlacesController.prototype = {
     var selectedNode = this._view.selectedNode;
     var txn = PlacesUtils.ptm.sortFolderByName(selectedNode.itemId,
                                                selectedNode.bookmarkIndex);
-    PlacesUtils.ptm.commitTransaction(txn);
+    PlacesUtils.ptm.doTransaction(txn);
   },
 
   /**
@@ -899,7 +899,7 @@ PlacesController.prototype = {
       this._removeRange(ranges[i], transactions);
     if (transactions.length > 0) {
       var txn = PlacesUtils.ptm.aggregateTransactions(txnName, transactions);
-      PlacesUtils.ptm.commitTransaction(txn);
+      PlacesUtils.ptm.doTransaction(txn);
     }
   },
 
@@ -1159,7 +1159,7 @@ PlacesController.prototype = {
                                         PlacesUtils.TYPE_X_MOZ_URL, 
                                         PlacesUtils.TYPE_UNICODE]);
     var txn = PlacesUtils.ptm.aggregateTransactions("Paste", transactions);
-    PlacesUtils.ptm.commitTransaction(txn);
+    PlacesUtils.ptm.doTransaction(txn);
   }
 };
 
@@ -1586,7 +1586,7 @@ var PlacesControllerDragHelper = {
     }
 
     var txn = PlacesUtils.ptm.aggregateTransactions("DropItems", transactions);
-    PlacesUtils.ptm.commitTransaction(txn);
+    PlacesUtils.ptm.doTransaction(txn);
   }
 };
 

browser/components/places/content/editBookmarkOverlay.js

@@ -496,7 +496,7 @@ var gEditItemOverlay = {
     }
 
     var aggregate = ptm.aggregateTransactions("Edit Item Title", txns);
-    ptm.commitTransaction(aggregate);
+    ptm.doTransaction(aggregate);
   },
 
   onDescriptionFieldBlur: function EIO_onDescriptionFieldInput() {
@@ -504,7 +504,7 @@ var gEditItemOverlay = {
     if (description != PlacesUtils.getItemDescription(this._itemId)) {
       var txn = PlacesUtils.ptm
                            .editItemDescription(this._itemId, description);
-      PlacesUtils.ptm.commitTransaction(txn);
+      PlacesUtils.ptm.doTransaction(txn);
     }
   },
 
@@ -517,7 +517,7 @@ var gEditItemOverlay = {
 
     if (!this._uri.equals(uri)) {
       var txn = PlacesUtils.ptm.editBookmarkURI(this._itemId, uri);
-      PlacesUtils.ptm.commitTransaction(txn);
+      PlacesUtils.ptm.doTransaction(txn);
     }
   },
 
@@ -525,7 +525,7 @@ var gEditItemOverlay = {
     var keyword = this._element("keywordField").value;
     if (keyword != PlacesUtils.bookmarks.getKeywordForBookmark(this._itemId)) {
       var txn = PlacesUtils.ptm.editBookmarkKeyword(this._itemId, keyword);
-      PlacesUtils.ptm.commitTransaction(txn);
+      PlacesUtils.ptm.doTransaction(txn);
     }
   },
 
@@ -539,7 +539,7 @@ var gEditItemOverlay = {
     var currentFeedURI = PlacesUtils.livemarks.getFeedURI(this._itemId);
     if (!currentFeedURI.equals(uri)) {
       var txn = PlacesUtils.ptm.editLivemarkFeedURI(this._itemId, uri);
-      PlacesUtils.ptm.commitTransaction(txn);
+      PlacesUtils.ptm.doTransaction(txn);
     }
   },
 
@@ -553,7 +553,7 @@ var gEditItemOverlay = {
     var currentSiteURI = PlacesUtils.livemarks.getSiteURI(this._itemId);
     if (!uri || !currentSiteURI.equals(uri)) {
       var txn = PlacesUtils.ptm.editLivemarkSiteURI(this._itemId, uri);
-      PlacesUtils.ptm.commitTransaction(txn);
+      PlacesUtils.ptm.doTransaction(txn);
     }
   },
 
@@ -562,7 +562,7 @@ var gEditItemOverlay = {
     var loadInSidebarChecked = this._element("loadInSidebarCheckbox").checked;
     var txn = PlacesUtils.ptm.setLoadInSidebar(this._itemId,
                                                loadInSidebarChecked);
-    PlacesUtils.ptm.commitTransaction(txn);
+    PlacesUtils.ptm.doTransaction(txn);
   },
 
   toggleFolderTreeVisibility: function EIO_toggleFolderTreeVisibility() {
@@ -652,7 +652,7 @@ var gEditItemOverlay = {
     // Move the item
     if (PlacesUtils.bookmarks.getFolderIdForItem(this._itemId) != container) {
       var txn = PlacesUtils.ptm.moveItem(this._itemId, container, -1);
-      PlacesUtils.ptm.commitTransaction(txn);
+      PlacesUtils.ptm.doTransaction(txn);
 
       // Mark the containing folder as recently-used if it isn't the
       // "All Bookmarks" root

browser/components/places/content/moveBookmarks.js

@@ -38,7 +38,6 @@
 
 var gMoveBookmarksDialog = {
   _nodes: null,
-  _tm: null,
 
   _foldersTree: null,
   get foldersTree() {
@@ -50,7 +49,6 @@ var gMoveBookmarksDialog = {
 
   init: function() {
     this._nodes = window.arguments[0];
-    this._tm = window.arguments[1];
 
     this.foldersTree.place =
       "place:excludeItems=1&excludeQueries=1&excludeReadOnlyFolders=1&folder=" +
@@ -75,7 +73,7 @@ var gMoveBookmarksDialog = {
 
     if (transactions.length != 0) {
       var txn = PlacesUtils.ptm.aggregateTransactions("Move Items", transactions);
-      this._tm.doTransaction(txn);
+      PlacesUtils.ptm.doTransaction(txn);
     }
   },
 

browser/components/places/content/places.js

@@ -627,7 +627,7 @@ var PlacesOrganizer = {
                                          PlacesUtils.bookmarksMenuFolderId,
                                          PlacesUtils.bookmarks.DEFAULT_INDEX,
                                          input.value);
-    PlacesUtils.ptm.commitTransaction(txn);
+    PlacesUtils.ptm.doTransaction(txn);
   }
 };
 

browser/components/places/content/utils.js

@@ -167,11 +167,6 @@ var PlacesUtils = {
     return this.localStore = this.RDF.GetDataSource("rdf:local-store");
   },
 
-  get tm() {
-    delete this.tm;
-    return this.tm = this.ptm.transactionManager;
-  },
-
   get ptm() {
     delete this.ptm;
     return this.ptm = Cc["@mozilla.org/browser/placesTransactionsService;1"].

browser/components/places/public/nsIPlacesTransactionsService.idl

@@ -38,11 +38,11 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsISupports.idl"
+#include "nsITransactionManager.idl"
 
 interface nsIVariant;
 interface nsIURI;
 interface nsIMicrosummary;
-interface nsITransactionManager;
 interface nsITransaction;
 
 /**
@@ -52,17 +52,9 @@ interface nsITransaction;
  * the global scope of a js window.
  */
 
-[scriptable, uuid(34cfb73e-ecd4-434d-b0f0-c17b8c7ceae9)]
-interface nsIPlacesTransactionsService : nsISupports
+[scriptable, uuid(939bccbd-ecb1-4742-9c38-a33af91ec872)]
+interface nsIPlacesTransactionsService : nsITransactionManager
 {
-  /**
-   * Performs a transaction.
-   *
-   * @param aTransaction
-   *        a transaction object for a transaction
-   */
-  void commitTransaction(in nsITransaction aTransaction);
-
   /**
    * Transaction for performing several Places Transactions in a single batch. 
    * 
@@ -305,9 +297,4 @@ interface nsIPlacesTransactionsService : nsISupports
    */
   nsITransaction sortFolderByName(in long long aFolderId,
                                   in long long aFolderIndex);
-
-  /**
-   * A reference to the transaction manager
-   */
-  readonly attribute nsITransactionManager transactionManager;
 };

browser/components/places/src/nsPlacesTransactionsService.js

@@ -63,7 +63,8 @@ placesTransactionsService.prototype = {
   classID: CLASS_ID,
   contractID: CONTRACT_ID,
 
-  QueryInterface: XPCOMUtils.generateQI([Ci.nsIPlacesTransactionsService]),
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIPlacesTransactionsService,
+                                         Ci.nsITransactionManager]),
 
   aggregateTransactions: function placesAggrTransactions(name, transactions) {
     return new placesAggregateTransactions(name, transactions);
@@ -107,7 +108,7 @@ placesTransactionsService.prototype = {
     return new placesEditBookmarkURITransactions(aBookmarkId, aNewURI);
   },
 
-  setLoadInSidebar:  function placesSetLdInSdbar(aBookmarkId, aLoadInSidebar) {
+  setLoadInSidebar: function placesSetLdInSdbar(aBookmarkId, aLoadInSidebar) {
     return new placesSetLoadInSidebarTransactions(aBookmarkId, aLoadInSidebar);
   },
 
@@ -139,13 +140,57 @@ placesTransactionsService.prototype = {
    return new placesSortFolderByNameTransactions(aFolderId, aFolderIndex);
   },
 
-  commitTransaction: function placesCommitTxn(txn) {
-    this.mTransactionManager.doTransaction(txn);
+  // Update commands in the undo group of the active window
+  // commands in inactive windows will are updated on-focus
+  _updateCommands: function() {
+    var wm = Cc["@mozilla.org/appshell/window-mediator;1"].
+             getService(Ci.nsIWindowMediator);
+    var win = wm.getMostRecentWindow(null);
+    if (win)
+      win.updateCommands("undo");
   },
 
-  get transactionManager() {
-    return this.mTransactionManager;
-  }
+  // nsITransactionManager
+  doTransaction: function doTransaction(txn) {
+    this.mTransactionManager.doTransaction(txn);
+    this._updateCommands();
+  },
+
+  undoTransaction: function undoTransaction() {
+    this.mTransactionManager.undoTransaction();
+    this._updateCommands();
+  },
+
+  redoTransaction: function redoTransaction() {
+    this.mTransactionManager.redoTransaction();
+    this._updateCommands();
+  },
+
+  clear: function() this.mTransactionManager.clear(),
+  beginBatch: function() this.mTransactionManager.beginBatch(),
+  endBatch: function() this.mTransactionManager.endBatch(),
+
+  get numberOfUndoItems() {
+    return this.mTransactionManager.numberOfUndoItems;
+  },
+
+  get numberOfRedoItems() {
+    return this.mTransactionManager.numberOfRedoItems;
+  },
+
+  get maxTransactionCount() {
+    return this.mTransactionManager.maxTransactionCount;
+  },
+  set maxTransactionCount(val) {
+    return this.mTransactionManager.maxTransactionCount = val;
+  },
+
+  peekUndoStack: function() this.mTransactionManager.peekUndoStack(),
+  peekRedoStack: function() this.mTransactionManager.peekRedoStack(),
+  getUndoStack: function() this.mTransactionManager.getUndoStack(),
+  getRedoStack: function() this.mTransactionManager.getRedoStack(),
+  AddListener: function(l) this.mTransactionManager.AddListener(l),
+  RemoveListener: function(l) this.mTransactionManager.RemoveListener(l)
 };
 
 /**

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

@@ -148,7 +148,7 @@ function run_test() {
   // Test creating an item
   // Create to Root
   var txn2 = ptSvc.createItem(uri("http://www.example.com"), root, bmStartIndex, "Testing1");
-  ptSvc.commitTransaction(txn2); //Also testing commitTransaction
+  ptSvc.doTransaction(txn2); //Also testing doTransaction
   var b = (bmsvc.getBookmarkIdsForURI(uri("http://www.example.com"), {}))[0];
   do_check_eq(observer._itemAddedId, b);
   do_check_eq(observer._itemAddedIndex, bmStartIndex);
@@ -161,7 +161,7 @@ function run_test() {
   var txn2a = ptSvc.createFolder("Folder", root, bmStartIndex);
   var fldrId = bmsvc.getChildFolder(root, "Folder");
   var txn2b = ptSvc.createItem(uri("http://www.example2.com"), fldrId, bmStartIndex, "Testing1b");
-  ptSvc.commitTransaction(txn2b);
+  ptSvc.doTransaction(txn2b);
   var b2 = (bmsvc.getBookmarkIdsForURI(uri("http://www.example2.com"), {}))[0];
   do_check_eq(observer._itemAddedId, b2);
   do_check_eq(observer._itemAddedIndex, bmStartIndex);
@@ -171,9 +171,9 @@ function run_test() {
   do_check_eq(observer._itemRemovedIndex, bmStartIndex);
 
   // Testing moving an item
-  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.example3.com"), root, -1, "Testing2"));
-  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.example3.com"), root, -1, "Testing3"));   
-  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.example3.com"), fldrId, -1, "Testing4"));
+  ptSvc.doTransaction(ptSvc.createItem(uri("http://www.example3.com"), root, -1, "Testing2"));
+  ptSvc.doTransaction(ptSvc.createItem(uri("http://www.example3.com"), root, -1, "Testing3"));   
+  ptSvc.doTransaction(ptSvc.createItem(uri("http://www.example3.com"), fldrId, -1, "Testing4"));
   var bkmkIds = bmsvc.getBookmarkIdsForURI(uri("http://www.example3.com"), {});
   bkmkIds.sort();
   var bkmk1Id = bkmkIds[0];
@@ -211,7 +211,7 @@ function run_test() {
   do_check_eq(observer._itemMovedNewIndex, 0);
 
   // Test Removing a Folder
-  ptSvc.commitTransaction(ptSvc.createFolder("Folder2", root, -1));
+  ptSvc.doTransaction(ptSvc.createFolder("Folder2", root, -1));
   var fldrId2 = bmsvc.getChildFolder(root, "Folder2");
   var txn4 = ptSvc.removeItem(fldrId2);
   txn4.doTransaction();
@@ -246,7 +246,7 @@ function run_test() {
   do_check_eq(observer._itemRemovedIndex, 1);
 
   // Test removing a separator
-  ptSvc.commitTransaction(ptSvc.createSeparator(root, 1));
+  ptSvc.doTransaction(ptSvc.createSeparator(root, 1));
   var sepId2 = observer._itemAddedId;
   var txn7 = ptSvc.removeItem(sepId2);
   txn7.doTransaction();
@@ -342,11 +342,11 @@ function run_test() {
   do_check_eq(observer._itemChanged_isAnnotationProperty, true);
 
   // sortFolderByName
-  ptSvc.commitTransaction(ptSvc.createFolder("Sorting folder", root, bmStartIndex, [], null));
+  ptSvc.doTransaction(ptSvc.createFolder("Sorting folder", root, bmStartIndex, [], null));
   var srtFldId = bmsvc.getChildFolder(root, "Sorting folder");
-  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.sortingtest.com"), srtFldId, -1, "c"));
-  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.sortingtest.com"), srtFldId, -1, "b"));   
-  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.sortingtest.com"), srtFldId, -1, "a"));
+  ptSvc.doTransaction(ptSvc.createItem(uri("http://www.sortingtest.com"), srtFldId, -1, "c"));
+  ptSvc.doTransaction(ptSvc.createItem(uri("http://www.sortingtest.com"), srtFldId, -1, "b"));   
+  ptSvc.doTransaction(ptSvc.createItem(uri("http://www.sortingtest.com"), srtFldId, -1, "a"));
   var b = bmsvc.getBookmarkIdsForURI(uri("http://www.sortingtest.com"), {});
   b.sort();
   var b1 = b[0];
@@ -368,7 +368,7 @@ function run_test() {
   // editBookmarkMicrosummary
   var tmpMs = mss.createMicrosummary(uri("http://testmicro.com"), 
                                      uri("http://dietrich.ganx4.com/mozilla/test-microsummary.xml"));
-  ptSvc.commitTransaction(
+  ptSvc.doTransaction(
   ptSvc.createItem(uri("http://dietrich.ganx4.com/mozilla/test-microsummary-content.php"),
                    root, -1, "micro test", null, null, null));
   var bId = (bmsvc.getBookmarkIdsForURI(uri("http://dietrich.ganx4.com/mozilla/test-microsummary-content.php"),{}))[0];