Bug 1921811 - Make URLBar a popover. r=emilio,desktop-theme-reviewers…

…,tabbrowser-reviewers,sidebar-reviewers,urlbar-reviewers,dao,mak,dao? * Fix various z-index/paint order issues by making the urlbar a popover - which moves the open urlbar to the top layer, outside the toolbox * Adjust the urlbar position when the autohide menubar shows and hides Combines work from nsharpley and sfoster on top of emilio's original popover patch Differential Revision: https://phabricator.services.mozilla.com/D224201
mozilla · Oct 21, 2024 · eca15e4 · eca15e4
1 parent 0a2aac0
commit eca15e4
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 32 deletions.
diff --git a/browser/base/content/navigator-toolbox.inc.xhtml b/browser/base/content/navigator-toolbox.inc.xhtml
@@ -163,7 +163,8 @@
                    removable="false"
                    overflows="false">
         <toolbartabstop/>
-        <hbox id="urlbar" flex="1"
+        <html:div id="urlbar"
+              popover="manual"
               context=""
               focused="true"
               pageproxystate="invalid">
@@ -416,7 +417,7 @@
               </hbox>
             </hbox>
           </hbox>
-        </hbox>
+        </html:div>
         <toolbartabstop/>
       </toolbaritem>
 

diff --git a/browser/components/urlbar/UrlbarInput.sys.mjs b/browser/components/urlbar/UrlbarInput.sys.mjs
@@ -272,6 +272,24 @@ export class UrlbarInput {
     this.window.addEventListener("customizationstarting", this);
     this.window.addEventListener("aftercustomization", this);
 
+    const menubar = this.window.document.getElementById("toolbar-menubar");
+    if (menubar) {
+      menubar.addEventListener("DOMMenuBarInactive", this);
+      menubar.addEventListener("DOMMenuBarActive", this);
+    }
+
+    if (this._toolbar) {
+      // TODO(emilio): This could use CSS anchor positioning rather than this
+      // ResizeObserver, eventually.
+      let observer = new this.window.ResizeObserver(([entry]) => {
+        this.textbox.style.setProperty(
+          "--urlbar-width",
+          px(entry.borderBoxSize[0].inlineSize)
+        );
+      });
+      observer.observe(this.textbox.parentNode);
+    }
+
     this.updateLayoutBreakout();
 
     this._initCopyCutController();
@@ -2141,6 +2159,8 @@ export class UrlbarInput {
       return;
     }
 
+    this._updateTextboxPosition();
+
     if (Cu.isInAutomation) {
       if (lazy.UrlbarPrefs.get("disableExtendForTests")) {
         this.setAttribute("breakout-extend-disabled", "true");
@@ -2149,9 +2169,10 @@ export class UrlbarInput {
       this.removeAttribute("breakout-extend-disabled");
     }
 
-    this._toolbar.setAttribute("urlbar-exceeds-toolbar-bounds", "true");
     this.setAttribute("breakout-extend", "true");
 
+    this.textbox.showPopover();
+
     // Enable the animation only after the first extend call to ensure it
     // doesn't run when opening a new window.
     if (!this.hasAttribute("breakout-extend-animate")) {
@@ -2164,6 +2185,7 @@ export class UrlbarInput {
   }
 
   endLayoutExtend() {
+    this._updateTextboxPosition();
     // If reduce motion is enabled, we want to collapse the Urlbar here so the
     // user sees only sees two states: not expanded, and expanded with the view
     // open.
@@ -2172,7 +2194,6 @@ export class UrlbarInput {
     }
 
     this.removeAttribute("breakout-extend");
-    this._toolbar.removeAttribute("urlbar-exceeds-toolbar-bounds");
   }
 
   /**
@@ -2389,13 +2410,40 @@ export class UrlbarInput {
     this.view.close();
   }
 
+  _updateTextboxPosition() {
+    if (this.view.isOpen) {
+      // We need to adjust the position of the textbox by measuring its container
+      this.textbox.style.top = px(
+        getBoundsWithoutFlushing(this.textbox.parentNode).top
+      );
+    } else {
+      this.textbox.style.top = "";
+    }
+  }
+
+  _updateTextboxPositionNextFrame() {
+    // Allow for any layout changes to take place (e.g. when the menubar becomes
+    // inactive) before re-measuring to position the textbox
+    this.window.requestAnimationFrame(() => {
+      this.window.requestAnimationFrame(() => {
+        this._updateTextboxPosition();
+      });
+    });
+  }
+
   async _updateLayoutBreakoutDimensions() {
     // When this method gets called a second time before the first call
     // finishes, we need to disregard the first one.
     let updateKey = {};
     this._layoutBreakoutUpdateKey = updateKey;
 
     this.removeAttribute("breakout");
+    try {
+      this.textbox.hidePopover();
+    } catch (ex) {
+      // No big deal if not a popover already.
+    }
+
     this.textbox.parentNode.removeAttribute("breakout");
 
     await this.window.promiseDocumentFlushed(() => {});
@@ -4621,6 +4669,18 @@ export class UrlbarInput {
     this._initStripOnShare();
   }
 
+  _on_DOMMenuBarActive() {
+    if (this.hasAttribute("breakout")) {
+      this._updateTextboxPositionNextFrame();
+    }
+  }
+
+  _on_DOMMenuBarInactive() {
+    if (this.hasAttribute("breakout")) {
+      this._updateTextboxPositionNextFrame();
+    }
+  }
+
   #allTextSelectedOnKeyDown = false;
   get #allTextSelected() {
     return this.selectionStart == 0 && this.selectionEnd == this.value.length;

diff --git a/browser/themes/shared/browser-shared.css b/browser/themes/shared/browser-shared.css
@@ -63,7 +63,10 @@ body {
   --browser-stack-z-index-rdm-toolbar: 3;
 
   /* z-indices that fight in the general browser / toolbox area */
-  --browser-area-z-index-toolbox: 1;
+  --browser-area-z-index-toolbox: 0;
+  --browser-area-z-index-sidebar: 1;
+  --browser-area-z-index-tabbox: 2;
+  --browser-area-z-index-sidebar-splitter: 3;
 
   --toolbarbutton-border-radius: 4px;
   --identity-box-margin-inline: 4px;
@@ -125,12 +128,6 @@ body {
 #navigator-toolbox {
   appearance: none;
   position: relative;
-
-  /*
-   Should paint on top of our sibling #browser (even when preceding it in the DOM)
-   TODO(sfoster): we need a broader re-think of stacking and paint order.
-   See bug 1921257 and bug 1921811.
-   */
   z-index: var(--browser-area-z-index-toolbox);
 
   background-color: var(--toolbox-non-lwt-bgcolor);
@@ -314,13 +311,6 @@ body {
     border-top-style: none;
   }
 
-  /* The address bar needs to be able to render outside of the toolbar, but as
-   * long as it's within the toolbar's bounds we can clip the toolbar so that the
-   * rendering pipeline doesn't reserve an enormous texture for it. */
-  &:not([urlbar-exceeds-toolbar-bounds]) {
-    overflow: clip;
-  }
-
   :root[sessionrestored][lwtheme] & {
     transition: var(--ext-theme-background-transition);
   }

diff --git a/browser/themes/shared/sidebar.css b/browser/themes/shared/sidebar.css
@@ -23,6 +23,7 @@
     padding-block-end: var(--space-small);
     padding-inline-end: var(--space-small);
     position: relative;
+    z-index: var(--browser-area-z-index-sidebar);
 
     &[positionend] {
       padding-inline-end: 0;
@@ -86,7 +87,7 @@
   --splitter-width: 4px;
   /* Ensure the splitter is painted on top of the sidebar box it overlaps.
   Otherwise, the user may be unable to drag the splitter to resize the sidebar. */
-  z-index: 1;
+  z-index: var(--browser-area-z-index-sidebar-splitter);
 
   /* stylelint-disable-next-line media-query-no-invalid */
   @media (-moz-bool-pref: "sidebar.revamp") or (not (-moz-platform: linux)) {

diff --git a/browser/themes/shared/tabbrowser/content-area.css b/browser/themes/shared/tabbrowser/content-area.css
@@ -46,6 +46,7 @@
 
 #tabbrowser-tabbox {
   position: relative;
+  z-index: var(--browser-area-z-index-tabbox);
   margin: 0;
 
   /* stylelint-disable-next-line media-query-no-invalid */

diff --git a/browser/themes/shared/urlbar-searchbar.css b/browser/themes/shared/urlbar-searchbar.css
@@ -44,15 +44,28 @@
   }
 }
 
+#urlbar {
+  display: flex;
+  flex-direction: row;
+  flex: 1;
+  box-sizing: border-box;
+
+  /* Reset UA popover rules */
+  width: initial;
+  height: initial;
+  inset: auto;
+  border: none;
+  padding: initial;
+  overflow: initial;
+  color: initial;
+  background-color: initial;
+}
+
 #urlbar,
 #searchbar {
   /* Setting a min-width to let the location & search bars maintain a constant
    * width in case they haven't been resized manually. (bug 965772) */
   min-width: 1px;
-}
-
-#urlbar,
-#searchbar {
   min-height: var(--urlbar-min-height);
   text-shadow: none;
   color: var(--toolbar-field-color);
@@ -248,15 +261,13 @@
 }
 
 #urlbar-container[breakout] {
-  position: relative;
   min-height: var(--urlbar-container-height);
 }
 
 #urlbar[breakout] {
   display: block;
-  position: absolute;
-  width: 100%;
   height: var(--urlbar-height);
+  width: var(--urlbar-width);
 
   > .urlbar-input-container {
     width: 100%;
@@ -271,16 +282,12 @@
 
 #urlbar[breakout][breakout-extend],
 #urlbar[breakout][breakout-extend-disabled][open] {
-  /* The z-index needs to be big enough to trump other positioned UI pieces
-     that we want to overlay. 3 is used in the tab bar. */
-  z-index: 3;
   height: auto;
 }
 
 #urlbar[breakout][breakout-extend] {
-  top: 0;
-  left: calc(-1 * var(--urlbar-margin-inline));
-  width: calc(100% + 2 * var(--urlbar-margin-inline));
+  margin-left: calc(-1 * var(--urlbar-margin-inline));
+  width: calc(var(--urlbar-width) + 2 * var(--urlbar-margin-inline));
 
   > .urlbar-input-container {
     height: var(--urlbar-container-height);
@@ -289,6 +296,14 @@
   }
 }
 
+#urlbar[breakout][breakout-extend-disabled][open] {
+  /*
+    Bug 1925712: To avoid flickering in perf tests, which apply the breakout-extend-disabled
+    rather than breakout-extended attribute, we need to add some top offset.
+  */
+  margin-top: 3px;
+}
+
 @keyframes urlbar-grow {
   0% {
     transform: scaleX(.99) scaleY(.98);