Bug 432599 - Double-click on the Star icon leads to incorrect display of the bookmark properties panel; r=mano
authorEhsan Akhgari <ehsan.akhgari@gmail.com>
Thu, 18 Dec 2008 23:42:40 +0330
changeset 22916 aa275166759524b70a867ceb83d62bdb0a10f1c8
parent 22915 d20515d542c9e5d3e90d86e6bc8940a7f4dbe99f
child 22917 54d08b93e78bc06e8731bcc1e2eebe215af9e7d7
push id4258
push userehsan.akhgari@gmail.com
push dateThu, 18 Dec 2008 20:14:05 +0000
treeherdermozilla-central@54d08b93e78b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmano
bugs432599
milestone1.9.2a1pre
Bug 432599 - Double-click on the Star icon leads to incorrect display of the bookmark properties panel; r=mano
browser/base/content/browser-places.js
browser/base/content/test/Makefile.in
browser/base/content/test/browser_bug432599.js
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -17,16 +17,17 @@
 # Portions created by the Initial Developer are Copyright (C) 2006
 # the Initial Developer. All Rights Reserved.
 #
 # Contributor(s):
 #   Ben Goodger <beng@google.com>
 #   Annie Sullivan <annie.sullivan@gmail.com>
 #   Joe Hughes <joe@retrovirus.com>
 #   Asaf Romano <mano@mozilla.com>
+#   Ehsan Akhgari <ehsan.akhgari@gmail.com>
 #
 # Alternatively, the contents of this file may be used under the terms of
 # either the GNU General Public License Version 2 or later (the "GPL"), or
 # the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
 # in which case the provisions of the GPL or the LGPL are applicable instead
 # of those above. If you wish to allow use of your version of this file only
 # under the terms of either the GPL or the LGPL, and not to allow others to
 # use your version of this file under the terms of the MPL, indicate your
@@ -161,16 +162,19 @@ var StarUI = {
     };
     this._overlayLoading = true;
     document.loadOverlay("chrome://browser/content/places/editBookmarkOverlay.xul",
                          loadObserver);
   },
 
   _doShowEditBookmarkPanel:
   function SU__doShowEditBookmarkPanel(aItemId, aAnchorElement, aPosition) {
+    if (this.panel.state != "closed")
+      return;
+
     this._blockCommands(); // un-done in the popuphiding handler
 
     var bundle = this._element("bundle_browser");
 
     // Set panel title:
     // if we are batching, i.e. the bookmark has been added now,
     // then show Page Bookmarked, else if the bookmark did already exist,
     // we are about editing it, then use Edit This Bookmark.
@@ -211,27 +215,20 @@ var StarUI = {
     // transaction on the undo stack may be the initial createItem transaction,
     // or worse, the batched editing of some other item.
     PlacesUIUtils.ptm.doTransaction({ doTransaction: function() { },
                                       undoTransaction: function() { },
                                       redoTransaction: function() { },
                                       isTransient: false,
                                       merge: function() { return false; } });
 
-    if (this.panel.state == "closed") {
-      // Consume dismiss clicks, see bug 400924
-      this.panel.popupBoxObject
-          .setConsumeRollupEvent(Ci.nsIPopupBoxObject.ROLLUP_CONSUME);
-      this.panel.openPopup(aAnchorElement, aPosition, -1, -1);
-    }
-    else {
-      var namePicker = this._element("editBMPanel_namePicker");
-      namePicker.focus();
-      namePicker.editor.selectAll();
-    }
+    // Consume dismiss clicks, see bug 400924
+    this.panel.popupBoxObject
+        .setConsumeRollupEvent(Ci.nsIPopupBoxObject.ROLLUP_CONSUME);
+    this.panel.openPopup(aAnchorElement, aPosition, -1, -1);
 
     gEditItemOverlay.initPanel(this._itemId,
                                { hiddenRows: ["description", "location",
                                               "loadInSidebar", "keyword"] });
   },
 
   panelShown:
   function SU_panelShown(aEvent) {
--- a/browser/base/content/test/Makefile.in
+++ b/browser/base/content/test/Makefile.in
@@ -55,16 +55,17 @@ include $(topsrcdir)/config/rules.mk
 # browser_bug423833.js disabled temporarily since it's unreliable: bug 428712
 # browser_sanitize-download-history.js disabled temporarily since it's unreliable: bug 432425
 _BROWSER_FILES = browser_bug321000.js \
                  browser_sanitize-timespans.js \
                  browser_bug405137.js \
                  browser_bug409481.js \
                  browser_bug413915.js \
                  browser_bug420160.js \
+                 browser_bug432599.js \
                  browser_bug427559.js \
                  browser_bug441778.js \
                  browser_discovery.js \
                  discovery.html \
                  moz.png \
                  browser_getshortcutoruri.js \
                  browser_page_style_menu.js \
                  page_style_sample.html \
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/browser_bug432599.js
@@ -0,0 +1,109 @@
+function invokeUsingCtrlD(phase) {
+  switch (phase) {
+  case 1:
+    EventUtils.synthesizeKey("d", { accelKey: true });
+    break;
+  case 2:
+  case 4:
+    EventUtils.synthesizeKey("VK_ESCAPE", {});
+    break;
+  case 3:
+    EventUtils.synthesizeKey("d", { accelKey: true });
+    EventUtils.synthesizeKey("d", { accelKey: true });
+    break;
+  }
+}
+
+function invokeUsingStarButton(phase) {
+  switch (phase) {
+  case 1:
+    EventUtils.sendMouseEvent({ type: "click" }, "star-button");
+    break;
+  case 2:
+  case 4:
+    EventUtils.synthesizeKey("VK_ESCAPE", {});
+    break;
+  case 3:
+    EventUtils.synthesizeMouse(document.getElementById("star-button"),
+                               1, 1, { clickCount: 2 });
+    break;
+  }
+}
+
+// test bug 432599
+function test() {
+  waitForExplicitFinish();
+
+  let testTab = gBrowser.addTab();
+  gBrowser.selectedTab = testTab;
+  let testBrowser = gBrowser.getBrowserForTab(testTab);
+  testBrowser.addEventListener("load", initTest, true);
+
+  testBrowser.contentWindow.location = "data:text/plain,Content";
+}
+
+let invokers = [invokeUsingStarButton, invokeUsingCtrlD];
+let currentInvoker = 0;
+
+function initTest() {
+  // first, bookmark the page
+  let app = Components.classes["@mozilla.org/fuel/application;1"]
+                      .getService(Components.interfaces.fuelIApplication);
+  let ios = Components.classes["@mozilla.org/network/io-service;1"]
+                      .getService(Ci.nsIIOService);
+  app.bookmarks.toolbar.addBookmark("Bug 432599 Test",
+                                    ios.newURI("data:text/plain,Content", null, null));
+
+  checkBookmarksPanel(invokers[currentInvoker], 1);
+}
+
+let initialValue;
+let initialRemoveHidden;
+
+let popupElement = document.getElementById("editBookmarkPanel");
+let titleElement = document.getElementById("editBookmarkPanelTitle");
+let removeElement = document.getElementById("editBookmarkPanelRemoveButton");
+
+function checkBookmarksPanel(invoker, phase)
+{
+  let onPopupShown = function(aEvent) {
+    if (aEvent.originalTarget == popupElement) {
+      checkBookmarksPanel(invoker, phase + 1);
+      popupElement.removeEventListener("popupshown", onPopupShown, false);
+    }
+  };
+  let onPopupHidden = function(aEvent) {
+    if (aEvent.originalTarget == popupElement) {
+      if (phase < 4) {
+        checkBookmarksPanel(invoker, phase + 1);
+      } else {
+        ++ currentInvoker;
+        if (currentInvoker < invokers.length) {
+          checkBookmarksPanel(invokers[currentInvoker], 1);
+        } else {
+          gBrowser.removeCurrentTab();
+          finish();
+        }
+      }
+      popupElement.removeEventListener("popuphidden", onPopupHidden, false);
+    }
+  };
+
+  switch (phase) {
+  case 1:
+  case 3:
+    popupElement.addEventListener("popupshown", onPopupShown, false);
+    break;
+  case 2:
+    popupElement.addEventListener("popuphidden", onPopupHidden, false);
+    initialValue = titleElement.value;
+    initialRemoveHidden = removeElement.hidden;
+    break;
+  case 4:
+    popupElement.addEventListener("popuphidden", onPopupHidden, false);
+    is(titleElement.value, initialValue, "The bookmark panel's title should be the same");
+    is(removeElement.hidden, initialRemoveHidden, "The bookmark panel's visibility should not change");
+    break;
+  }
+  invoker(phase);
+}