From 623682963287a6a6d0e6ae8af1ddeb4c0070604f Mon Sep 17 00:00:00 2001
From: Tim Nguyen <ntim.bugs@gmail.com>
Date: Mon, 27 Jul 2020 09:23:53 +0000
Subject: [PATCH] Bug 1624599 - When menu arrow scroll buttons overlap the menu
 contents, use overflow:hidden to clip away the contents underneath, rather
 than relying on -moz-appearance tricks. r=dao

Differential Revision: https://phabricator.services.mozilla.com/D80036
---
 toolkit/content/widgets/arrowscrollbox.js |  9 +++--
 toolkit/content/widgets/menupopup.js      |  4 +++
 toolkit/themes/osx/global/menu.css        | 44 +++++++++++------------
 3 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/toolkit/content/widgets/arrowscrollbox.js b/toolkit/content/widgets/arrowscrollbox.js
index d081ecd2b823..eda0c37d0f75 100644
--- a/toolkit/content/widgets/arrowscrollbox.js
+++ b/toolkit/content/widgets/arrowscrollbox.js
@@ -15,6 +15,7 @@
     static get inheritedAttributes() {
       return {
         "#scrollbutton-up": "disabled=scrolledtostart",
+        ".scrollbox-clip": "orient",
         scrollbox: "orient,align,pack,dir,smoothscroll",
         "#scrollbutton-down": "disabled=scrolledtoend",
       };
@@ -26,9 +27,11 @@
       <html:link rel="stylesheet" href="chrome://global/skin/arrowscrollbox.css"/>
       <toolbarbutton id="scrollbutton-up" part="scrollbutton-up"/>
       <spacer part="overflow-start-indicator"/>
-      <scrollbox part="scrollbox" flex="1">
-        <html:slot/>
-      </scrollbox>
+      <box class="scrollbox-clip" part="scrollbox-clip" flex="1">
+        <scrollbox part="scrollbox" flex="1">
+          <html:slot/>
+        </scrollbox>
+      </box>
       <spacer part="overflow-end-indicator"/>
       <toolbarbutton id="scrollbutton-down" part="scrollbutton-down"/>
     `;
diff --git a/toolkit/content/widgets/menupopup.js b/toolkit/content/widgets/menupopup.js
index d46562f8272f..307cc584c7e3 100644
--- a/toolkit/content/widgets/menupopup.js
+++ b/toolkit/content/widgets/menupopup.js
@@ -98,6 +98,10 @@
         }
         :host(.in-menulist) arrowscrollbox::part(scrollbox) {
           overflow: auto;
+          margin: 0;
+        }
+        :host(.in-menulist) arrowscrollbox::part(scrollbox-clip) {
+          overflow: visible;
         }
       `;
 
diff --git a/toolkit/themes/osx/global/menu.css b/toolkit/themes/osx/global/menu.css
index 6ea04f124f53..962240480d64 100644
--- a/toolkit/themes/osx/global/menu.css
+++ b/toolkit/themes/osx/global/menu.css
@@ -171,31 +171,31 @@ menuseparator {
 
 /* Scroll buttons */
 
-.menupopup-arrowscrollbox:not(:-moz-lwtheme)::part(scrollbutton-up),
-.menupopup-arrowscrollbox:not(:-moz-lwtheme)::part(scrollbutton-down) {
-  position: relative;
-  /* Here we're using a little magic.
-   * The arrow button is supposed to overlay the scrollbox, blocking
-   * everything under it from reaching the screen. However, the menu background
-   * is slightly transparent, so how can we block something completely without
-   * messing up the transparency? It's easy: The native theming of the
-   * "menuitem" appearance uses CGContextClearRect before drawing, which
-   * clears everything under it.
-   * Without help from native theming this effect wouldn't be achievable.
-   */
-  appearance: auto;
-  -moz-default-appearance: menuitem;
+/* Hide arrow buttons when there's nothing to scroll in that direction */
+.menupopup-arrowscrollbox[scrolledtostart="true"]::part(scrollbutton-up),
+.menupopup-arrowscrollbox[scrolledtoend="true"]::part(scrollbutton-down) {
+  visibility: collapse;
 }
 
-.menupopup-arrowscrollbox:not(:-moz-lwtheme)::part(scrollbutton-up) {
-  margin-bottom: -16px;
-}
-
-.menupopup-arrowscrollbox:not(:-moz-lwtheme)::part(scrollbutton-down) {
+/* Prevent the scrolled contents of the menupopup from jumping vertically when
+ * the arrow buttons appear / disappear, by positioning ::part(scrollbox) in
+ * such a way that its edges are at the same position as the edges of
+ * arrowscrollbox regardless of scroll button visibility.
+ */
+.menupopup-arrowscrollbox:not([scrolledtostart="true"])::part(scrollbox) {
+  /* scrollbutton-up is visible; shift our top edge up by its height. */
   margin-top: -16px;
 }
 
-.menupopup-arrowscrollbox:not(:-moz-lwtheme)[scrolledtostart="true"]::part(scrollbutton-up),
-.menupopup-arrowscrollbox:not(:-moz-lwtheme)[scrolledtoend="true"]::part(scrollbutton-down) {
-  visibility: collapse;
+.menupopup-arrowscrollbox:not([scrolledtoend="true"])::part(scrollbox) {
+  /* scrollbutton-down is visible; shift our bottom edge down by its height. */
+  margin-bottom: -16px;
+}
+
+.menupopup-arrowscrollbox::part(scrollbox-clip) {
+  /* In the space where the arrow buttons overlap the scrollbox, clip away the
+   * scrollbox so that nothing is shown behind the arrow button even if the
+   * button is transparent.
+   */
+  overflow: hidden;
 }