Bug 1446961, add two special cases in PopupBoxObject as supported in popup.xml, r=paolo,bz
authorNeil Deakin <neil@mozilla.com>
Fri, 27 Apr 2018 11:04:37 -0400
changeset 472225 12e768652a88d11b9c09ec63e1a5df00cdab4dc2
parent 472224 46d7f3cc71027fd301be83c9d462b94cdcf27f7b
child 472226 19b49df2389f777dd45779d89a3f83046c40b3d6
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaolo, bz
bugs1446961
milestone61.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 1446961, add two special cases in PopupBoxObject as supported in popup.xml, r=paolo,bz 1 - the old showPopup redirects to showMenu when no argument is specified. Support this in openPopup as bookmarks drag code that pops open menus as you drag over them relies on this 2 - reposition the popup after popup is resized, needed to update the arrow position in the arrow panel
layout/xul/PopupBoxObject.cpp
toolkit/content/tests/chrome/popup_trigger.js
--- a/layout/xul/PopupBoxObject.cpp
+++ b/layout/xul/PopupBoxObject.cpp
@@ -68,16 +68,27 @@ PopupBoxObject::OpenPopup(Element* aAnch
     aTriggerEvent = options.mTriggerEvent;
   }
   else {
     position = aOptions.GetAsString();
   }
 
   nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
   if (pm && mContent) {
+    // As a special case for popups that are menus when no anchor or position are
+    // specified, open the popup with ShowMenu instead of ShowPopup so that the
+    // popup is aligned with the menu.
+    if (!aAnchorElement && position.IsEmpty() && mContent->GetPrimaryFrame()) {
+      nsMenuFrame* menu = do_QueryFrame(mContent->GetPrimaryFrame()->GetParent());
+      if (menu) {
+        pm->ShowMenu(menu->GetContent(), false, false);
+        return;
+      }
+    }
+
     nsCOMPtr<nsIContent> anchorContent(do_QueryInterface(aAnchorElement));
     pm->ShowPopup(mContent, anchorContent, position, aXPos, aYPos,
                   aIsContextMenu, aAttributesOverride, false, aTriggerEvent);
   }
 }
 
 void
 PopupBoxObject::OpenPopupAtScreen(int32_t aXPos, int32_t aYPos,
@@ -144,16 +155,23 @@ PopupBoxObject::SizeTo(int32_t aWidth, i
 
   // We only want to pass aNotify=true to SetAttr once, but must make sure
   // we pass it when a value is being changed.  Thus, we check if the height
   // is the same and if so, pass true when setting the width.
   bool heightSame = element->AttrValueIs(kNameSpaceID_None, nsGkAtoms::height, height, eCaseMatters);
 
   element->SetAttr(kNameSpaceID_None, nsGkAtoms::width, width, heightSame);
   element->SetAttr(kNameSpaceID_None, nsGkAtoms::height, height, true);
+
+  // If the popup is open, force a reposition of the popup after resizing it
+  // with notifications set to true so that the popuppositioned event is fired.
+  nsMenuPopupFrame *menuPopupFrame = do_QueryFrame(mContent->GetPrimaryFrame());
+  if (menuPopupFrame && menuPopupFrame->PopupState() == ePopupShown) {
+    menuPopupFrame->SetPopupPosition(nullptr, false, false, true);
+  }
 }
 
 bool
 PopupBoxObject::AutoPosition()
 {
   nsMenuPopupFrame *menuPopupFrame = mContent ? do_QueryFrame(mContent->GetPrimaryFrame()) : nullptr;
   if (menuPopupFrame) {
     return menuPopupFrame->GetAutoPosition();
--- a/toolkit/content/tests/chrome/popup_trigger.js
+++ b/toolkit/content/tests/chrome/popup_trigger.js
@@ -803,16 +803,33 @@ var popupTests = [
   test(testname, step) {
     gMenuPopup.openPopup(gTrigger, { position: "after_start", x: 0, y: 0,
                                      triggerEvent: new MouseEvent("mousedown", { altKey: true })
                                     });
     checkOpen("trigger", testname);
   }
 },
 {
+  // openPopup with no arguments
+  testname: "openPopup with no arguments",
+  events: [ "popupshowing thepopup", "popupshown thepopup" ],
+  autohide: "thepopup",
+  test(testname, step) {
+    gMenuPopup.openPopup();
+  },
+  result(testname, step) {
+    let isMenu = gTrigger.type == "menu";
+    // With no arguments, open in default menu position
+    var triggerrect = gTrigger.getBoundingClientRect();
+    var popuprect = gMenuPopup.getBoundingClientRect();
+    is(Math.round(popuprect.left), isMenu ? Math.round(triggerrect.left) : 0, testname + " x position ");
+    is(Math.round(popuprect.top), isMenu ? Math.round(triggerrect.bottom) : 0, testname + " y position ");
+  }
+},
+{
   // openPopup should open the menu synchronously, however popupshown
   // is fired asynchronously
   testname: "openPopup synchronous",
   events: [ "popupshowing thepopup", "popupshowing submenupopup",
             "popupshown thepopup", "DOMMenuItemActive submenu",
             "popupshown submenupopup" ],
   test(testname, step) {
     gMenuPopup.openPopup(gTrigger, "after_start", 0, 0, false, true);