Bug 1356587, update the gEditUIVisible flag when the panelUI is opened and closed so that the command updating optimization actually applies. Currently it never applies once the panel has been opened at least once, r=gijs
authorNeil Deakin <neil@mozilla.com>
Mon, 01 May 2017 10:42:33 -0400
changeset 404042 56b70ce1a2ab369dd11b86d13d2966f8173ca356
parent 404041 1cd959bc881b16253a87eb33a06b9fcc60c5c02b
child 404043 492530a9b80ac49ebd53a00a874e29951b48c619
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgijs
bugs1356587
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1356587, update the gEditUIVisible flag when the panelUI is opened and closed so that the command updating optimization actually applies. Currently it never applies once the panel has been opened at least once, r=gijs
browser/base/content/browser.js
browser/components/customizableui/content/panelUI.js
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_editcontrols_update.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -4297,36 +4297,55 @@ function BrowserCustomizeToolbar() {
 function updateEditUIVisibility() {
   if (AppConstants.platform == "macosx")
     return;
 
   let editMenuPopupState = document.getElementById("menu_EditPopup").state;
   let contextMenuPopupState = document.getElementById("contentAreaContextMenu").state;
   let placesContextMenuPopupState = document.getElementById("placesContext").state;
 
+  let oldVisible = gEditUIVisible;
+
   // The UI is visible if the Edit menu is opening or open, if the context menu
   // is open, or if the toolbar has been customized to include the Cut, Copy,
   // or Paste toolbar buttons.
   gEditUIVisible = editMenuPopupState == "showing" ||
                    editMenuPopupState == "open" ||
                    contextMenuPopupState == "showing" ||
                    contextMenuPopupState == "open" ||
                    placesContextMenuPopupState == "showing" ||
-                   placesContextMenuPopupState == "open" ||
-                   document.getElementById("edit-controls") ? true : false;
+                   placesContextMenuPopupState == "open";
+  if (!gEditUIVisible) {
+    // Now check the edit-controls toolbar buttons.
+    let placement = CustomizableUI.getPlacementOfWidget("edit-controls");
+    let areaType = placement ? CustomizableUI.getAreaType(placement.area) : "";
+    if (areaType == CustomizableUI.TYPE_MENU_PANEL) {
+      let panelUIMenuPopupState = document.getElementById("PanelUI-popup").state;
+      if (panelUIMenuPopupState == "showing" || panelUIMenuPopupState == "open") {
+        gEditUIVisible = true;
+      }
+    } else if (areaType == CustomizableUI.TYPE_TOOLBAR) {
+      // The edit controls are on a toolbar, so they are visible.
+      gEditUIVisible = true;
+    }
+  }
+
+  // No need to update commands if the edit UI visibility has not changed.
+  if (gEditUIVisible == oldVisible) {
+    return;
+  }
 
   // If UI is visible, update the edit commands' enabled state to reflect
   // whether or not they are actually enabled for the current focus/selection.
-  if (gEditUIVisible)
+  if (gEditUIVisible) {
     goUpdateGlobalEditMenuItems();
-
-  // Otherwise, enable all commands, so that keyboard shortcuts still work,
-  // then lazily determine their actual enabled state when the user presses
-  // a keyboard shortcut.
-  else {
+  } else {
+    // Otherwise, enable all commands, so that keyboard shortcuts still work,
+    // then lazily determine their actual enabled state when the user presses
+    // a keyboard shortcut.
     goSetCommandEnabled("cmd_undo", true);
     goSetCommandEnabled("cmd_redo", true);
     goSetCommandEnabled("cmd_cut", true);
     goSetCommandEnabled("cmd_copy", true);
     goSetCommandEnabled("cmd_paste", true);
     goSetCommandEnabled("cmd_selectAll", true);
     goSetCommandEnabled("cmd_delete", true);
     goSetCommandEnabled("cmd_switchTextDirection", true);
--- a/browser/components/customizableui/content/panelUI.js
+++ b/browser/components/customizableui/content/panelUI.js
@@ -155,21 +155,16 @@ const PanelUI = {
     return new Promise(resolve => {
       this.ensureReady().then(() => {
         if (this.panel.state == "open" ||
             document.documentElement.hasAttribute("customizing")) {
           resolve();
           return;
         }
 
-        let editControlPlacement = CustomizableUI.getPlacementOfWidget("edit-controls");
-        if (editControlPlacement && editControlPlacement.area == CustomizableUI.AREA_PANEL) {
-          updateEditUIVisibility();
-        }
-
         let personalBookmarksPlacement = CustomizableUI.getPlacementOfWidget("personal-bookmarks");
         if (personalBookmarksPlacement &&
             personalBookmarksPlacement.area == CustomizableUI.AREA_PANEL) {
           PlacesToolbarHelper.customizeChange();
         }
 
         let anchor;
         if (!aEvent ||
@@ -287,20 +282,24 @@ const PanelUI = {
     // Ignore context menus and menu button menus showing and hiding:
     if (aEvent.type.startsWith("popup") &&
         aEvent.target != this.panel) {
       return;
     }
     switch (aEvent.type) {
       case "popupshowing":
         this._adjustLabelsForAutoHyphens();
+        updateEditUIVisibility();
         // Fall through
       case "popupshown":
         // Fall through
       case "popuphiding":
+        if (aEvent.type == "popuphiding") {
+          updateEditUIVisibility();
+        }
         // Fall through
       case "popuphidden":
         this._updateNotifications();
         this._updatePanelButton(aEvent.target);
         break;
       case "mousedown":
         if (aEvent.button == 0)
           this.toggle(aEvent);
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -150,8 +150,10 @@ skip-if = os == "mac"
 [browser_customizemode_contextmenu_menubuttonstate.js]
 [browser_exit_background_customize_mode.js]
 [browser_overflow_use_subviews.js]
 [browser_panel_toggle.js]
 [browser_panelUINotifications.js]
 [browser_switch_to_customize_mode.js]
 [browser_synced_tabs_menu.js]
 [browser_check_tooltips_in_navbar.js]
+[browser_editcontrols_update.js]
+subsuite = clipboard
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_editcontrols_update.js
@@ -0,0 +1,162 @@
+// This test checks that the edit command enabled state (cut/paste) is updated
+// properly when the edit controls are on the toolbar, popup and not present.
+// It also verifies that the performance optimiation implemented by
+// updateEditUIVisibility in browser.js is applied.
+
+let isMac = navigator.platform.indexOf("Mac") == 0;
+
+function checkState(allowCut, desc, testWindow = window) {
+  is(testWindow.document.getElementById("cmd_cut").getAttribute("disabled") == "true", !allowCut, desc + " - cut");
+  is(testWindow.document.getElementById("cmd_paste").getAttribute("disabled") == "true", false, desc + " - paste");
+}
+
+// Add a special controller to the urlbar and browser to listen in on when
+// commands are being updated. Return a promise that resolves when 'count'
+// updates have occurred.
+function expectCommandUpdate(count, testWindow = window) {
+  return new Promise((resolve, reject) => {
+    let overrideController = {
+      supportsCommand(cmd) { return cmd == "cmd_delete"; },
+      isCommandEnabled(cmd) {
+        if (!count) {
+          ok(false, "unexpected update");
+          reject();
+        }
+
+        if (!--count) {
+          testWindow.gURLBar.controllers.removeControllerAt(0, overrideController);
+          testWindow.gBrowser.selectedBrowser.controllers.removeControllerAt(0, overrideController);
+          resolve(true);
+        }
+      }
+    };
+
+    if (!count) {
+      SimpleTest.executeSoon(() => {
+        testWindow.gURLBar.controllers.removeControllerAt(0, overrideController);
+        testWindow.gBrowser.selectedBrowser.controllers.removeControllerAt(0, overrideController);
+        resolve(false);
+      });
+    }
+
+    testWindow.gURLBar.controllers.insertControllerAt(0, overrideController);
+    testWindow.gBrowser.selectedBrowser.controllers.insertControllerAt(0, overrideController);
+  });
+}
+
+add_task(function* test_init() {
+  // Put something on the clipboard to verify that the paste button is properly enabled during the test.
+  let clipboardHelper = Cc["@mozilla.org/widget/clipboardhelper;1"].getService(Ci.nsIClipboardHelper);
+  yield new Promise(resolve => {
+    SimpleTest.waitForClipboard("Sample", function() { clipboardHelper.copyString("Sample"); }, resolve);
+  });
+
+  // Open and close the panel first so that it is fully initialized.
+  yield PanelUI.show();
+  let hiddenPromise = promisePanelHidden(window);
+  PanelUI.hide();
+  yield hiddenPromise;
+});
+
+// Test updating when the panel is open with the edit-controls on the panel.
+// Updates should occur.
+add_task(function* test_panelui_opened() {
+  gURLBar.focus();
+  gURLBar.value = "test";
+
+  yield PanelUI.show();
+
+  checkState(false, "Update when edit-controls is on panel and visible");
+
+  let overridePromise = expectCommandUpdate(1);
+  gURLBar.select();
+  yield overridePromise;
+
+  checkState(true, "Update when edit-controls is on panel and selection changed");
+
+  overridePromise = expectCommandUpdate(0);
+  let hiddenPromise = promisePanelHidden(window);
+  PanelUI.hide();
+  yield hiddenPromise;
+  yield overridePromise;
+
+  // Check that updates do not occur after the panel has been closed.
+  checkState(true, "Update when edit-controls is on panel and hidden");
+
+  // Mac will update the enabled st1ate even when the panel is closed so that
+  // main menubar shortcuts will work properly.
+  overridePromise = expectCommandUpdate(isMac ? 1 : 0);
+  gURLBar.select();
+  yield overridePromise;
+  checkState(true, "Update when edit-controls is on panel, hidden and selection changed");
+});
+
+// Test updating when the edit-controls are moved to the toolbar.
+add_task(function* test_panelui_customize_to_toolbar() {
+  yield startCustomizing();
+  let navbar = document.getElementById("nav-bar").customizationTarget;
+  simulateItemDrag(document.getElementById("edit-controls"), navbar);
+  yield endCustomizing();
+
+  // updateEditUIVisibility should be called when customization ends but isn't. See bug 1359790.
+  updateEditUIVisibility();
+
+  let overridePromise = expectCommandUpdate(1);
+  gURLBar.select();
+  gURLBar.focus();
+  gURLBar.value = "other";
+  yield overridePromise;
+  checkState(false, "Update when edit-controls on toolbar and focused");
+
+  overridePromise = expectCommandUpdate(1);
+  gURLBar.select();
+  yield overridePromise;
+  checkState(true, "Update when edit-controls on toolbar and selection changed");
+});
+
+// Test updating when the edit-controls are moved to the palette.
+add_task(function* test_panelui_customize_to_palette() {
+  yield startCustomizing();
+  let palette = document.getElementById("customization-palette");
+  simulateItemDrag(document.getElementById("edit-controls"), palette);
+  yield endCustomizing();
+
+  // updateEditUIVisibility should be called when customization ends but isn't. See bug 1359790.
+  updateEditUIVisibility();
+
+  let overridePromise = expectCommandUpdate(isMac ? 1 : 0);
+  gURLBar.focus();
+  gURLBar.value = "other";
+  gURLBar.select();
+  yield overridePromise;
+
+  // If the UI isn't found, the command is set to be enabled.
+  checkState(true, "Update when edit-controls is on palette, hidden and selection changed");
+});
+
+add_task(function* finish() {
+  yield resetCustomization();
+});
+
+// Test updating in the initial state when the edit-controls are on the panel but
+// have not yet been created. This needs to be done in a new window to ensure that
+// other tests haven't opened the panel.
+add_task(function* test_initial_state() {
+  let testWindow = yield BrowserTestUtils.openNewBrowserWindow();
+  yield SimpleTest.promiseFocus(testWindow);
+
+  let overridePromise = expectCommandUpdate(isMac, testWindow);
+
+  testWindow.gURLBar.focus();
+  testWindow.gURLBar.value = "test";
+
+  yield overridePromise;
+
+  // Commands won't update when no edit UI is present. They default to being
+  // enabled so that keyboard shortcuts will work. The real enabled state will
+  // be checked when shortcut is pressed.
+  checkState(!isMac, "No update when edit-controls is on panel and not visible", testWindow);
+
+  yield BrowserTestUtils.closeWindow(testWindow);
+  yield SimpleTest.promiseFocus(window);
+});