Bug 1805500 - Implement panel borders in cocoa using a border rather than shadow. r=mstange,extension-reviewers

My guess is that it was done using shadows to not interfere with the
native look, but actually this just works even with native-looking menus
(like the <select> menulist), because the background-color for those is
set on the menupopup, rather than the ::part(content).

So those have effectively 1px of extra padding (due to the transparent
border), but that seems barely perceptible, and worth the consistency
and simplification.

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

gfx/layers/apz/test/mochitest/browser_test_select_popup_position.js

@@ -68,30 +68,42 @@ async function runPopupPositionTest(parentDocumentFileName) {
 
   const selectPopup = await openSelectPopup();
 
-  const popup_rect = selectPopup.getBoundingClientRect();
+  const popupRect = selectPopup.getBoundingClientRect();
   const popupMarginTop = parseFloat(getComputedStyle(selectPopup).marginTop);
   const popupMarginLeft = parseFloat(getComputedStyle(selectPopup).marginLeft);
 
+  info(
+    `popup rect: (${popupRect.x}, ${popupRect.y}) ${popupRect.width}x${popupRect.height}`
+  );
+  info(`popup margins: ${popupMarginTop} / ${popupMarginLeft}`);
+  info(
+    `select rect: (${selectRect.x}, ${selectRect.y}) ${selectRect.width}x${selectRect.height}`
+  );
+
   is(
-    popup_rect.left - popupMarginLeft,
+    popupRect.left - popupMarginLeft,
     selectRect.x * 2.0,
-    "select popup position should be scaled by the desktop zoom"
+    "select popup position x should be scaled by the desktop zoom"
   );
 
   // On platforms other than MaxOSX the popup menu is positioned below the
   // option element.
   if (!navigator.platform.includes("Mac")) {
     is(
-      popup_rect.top - popupMarginTop,
+      popupRect.top - popupMarginTop,
       tab.linkedBrowser.getBoundingClientRect().top +
         (selectRect.y + selectRect.height) * 2.0,
-      "select popup position should be scaled by the desktop zoom"
+      "select popup position y should be scaled by the desktop zoom"
     );
   } else {
+    // On mac it's aligned to the selected menulist option, so it should
+    // overlap the <select>, but we give it some wiggle room for
+    // paddings/margins between the labels and margins.
+    const spaceToFirstLabel = 5;
     is(
-      popup_rect.top - popupMarginTop,
+      popupRect.top - popupMarginTop + spaceToFirstLabel,
       tab.linkedBrowser.getBoundingClientRect().top + selectRect.y * 2.0,
-      "select popup position should be scaled by the desktop zoom"
+      "select popup position y should be scaled by the desktop zoom"
     );
   }
 

layout/xul/nsMenuPopupFrame.cpp

@@ -1211,16 +1211,9 @@ nsPoint nsMenuPopupFrame::AdjustPositionForAnchorAlign(nsRect& anchorRect,
     // around if its gets resized or the selection changed. Cache the value in
     // mPositionedOffset and use that instead for any future calculations.
     if (mIsOpenChanged || mReflowCallbackData.mIsOpenChanged) {
-      nsIFrame* selectedItemFrame = GetSelectedItemForAlignment();
-      if (selectedItemFrame) {
-        int32_t scrolly = 0;
-        nsIScrollableFrame* scrollframe = GetScrollFrame(this);
-        if (scrollframe) {
-          scrolly = scrollframe->GetScrollPosition().y;
-        }
-
-        mPositionedOffset = originalAnchorRect.height +
-                            selectedItemFrame->GetRect().y - scrolly;
+      if (nsIFrame* selectedItemFrame = GetSelectedItemForAlignment()) {
+        mPositionedOffset =
+            originalAnchorRect.height + selectedItemFrame->GetOffsetTo(this).y;
       }
     }
 

toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js

@@ -79,16 +79,7 @@ add_task(async function test_popup_styling(browser, accDoc) {
       );
 
       // Ensure popup border color was set properly
-      if (AppConstants.platform == "macosx") {
-        Assert.ok(
-          arrowContentComputedStyle
-            .getPropertyValue("box-shadow")
-            .includes(`rgb(${hexToRGB(POPUP_BORDER_COLOR).join(", ")})`),
-          "Popup border color should be set"
-        );
-      } else {
-        testBorderColor(arrowContent, POPUP_BORDER_COLOR);
-      }
+      testBorderColor(arrowContent, POPUP_BORDER_COLOR);
 
       await closeIdentityPopup();
       await extension.unload();

toolkit/content/tests/chrome/test_menulist_position.xhtml

@@ -51,13 +51,12 @@ function popupShown()
 
   let index = menulist.selectedIndex;
   if (menulist.selectedItem && navigator.platform.includes("Mac")) {
-    let menulistlabel = menulist.shadowRoot.getElementById("label");
-    let mitemlabel = menulist.selectedItem.querySelector(".menu-iconic-text");
+    let menulistlabelrect = menulist.shadowRoot.getElementById("label").getBoundingClientRect();
     let paddingTop = parseFloat(getComputedStyle(menulist.selectedItem).paddingTop);
+    let mitemlabelrect = menulist.selectedItem.querySelector(".menu-iconic-text").getBoundingClientRect();
 
-    ok(isWithinHalfPixel(menulistlabel.getBoundingClientRect().top + paddingTop,
-                         mitemlabel.getBoundingClientRect().top),
-       "Labels vertically aligned for index " + index);
+    ok(isWithinHalfPixel(menulistlabelrect.top - paddingTop, mitemlabelrect.top),
+       `Labels vertically aligned for ${index} : ${menulistlabelrect.top} - ${paddingTop} vs. ${mitemlabelrect.top}`);
 
     // Store the current value and reset it afterwards.
     let current = menulist.selectedIndex;

toolkit/themes/osx/global/popup.css

@@ -24,7 +24,7 @@ panel {
    * but as shadows on macOS are being taken care of by the OS, there's
    * no need for CSS to do calculations, so just set it to 0px here. */
   --panel-shadow-margin: 0px;
-  --panel-shadow: 0 0 0 1px var(--panel-border-color);
+  --panel-shadow: none;
 }
 
 menupopup > menu > menupopup {
@@ -56,7 +56,7 @@ panel[titlebar] {
   width: var(--panel-width);
   min-width: 0;
   min-height: 0;
-  border: none;
+  border: 1px solid var(--panel-border-color);
 }
 
 :is(panel, menupopup)[orient=vertical]::part(content) {