Bug 1461522 - Use the currentZoom as opposed to the pref value; r=jdescottes
authorBrian Birtles <birtles@gmail.com>
Thu, 28 Jun 2018 14:55:48 +0900
changeset 814146 52628cf763c9dcc0907b8bb54ee6ce82e92038a4
parent 814145 f995e3ff5de49405ece552c954514e568897d23c
child 814147 924dcb50f6da454045b6ca91ab20bd913396bec2
push id115123
push userjdescottes@mozilla.com
push dateWed, 04 Jul 2018 17:42:29 +0000
reviewersjdescottes
bugs1461522
milestone63.0a1
Bug 1461522 - Use the currentZoom as opposed to the pref value; r=jdescottes In a couple of places in DevTools we read back the value of the "devtools.toolbox.zoomValue" pref and take that to be the zoom value. However, at certain zoom levels these two can differ: the pref value representing the ideal zoom and currentZoom giving the actual zoom value in use. This patch makes us use the actual zoom value in the two places where this occurs. Unfortunately, we cannot easily adjust the browser_html_tooltip_zoom.js test for this since it uses a separate XUL document that does not take into account the pref value---as a result that test directly sets the currentZoom on the separate doc and hence this problem won't occur. Instead, this patch adjusts the browser_toolbox_zoom_popup.js test since the toolbox menu positioning uses the same problematic pattern so we can reproduce the bug in the browser_toolbox_zoom_popup.js. (This patch fixes both occurances of this pattern.) At least locally browser_toolbox_zoom_popup.js passes for me with a zoom of 1.5 but fails with a zoom of 1.4. Similarly in my testing of HTMLTooltip, zoom values such as 1.2 and 1.4 often show significant misalignment whilst a zoom of 1.3 or 1.5 is fine. With the code changes in this patch, the test passes with any given zoom factor. (This patch also incidentally replaced isNaN with the more robust Number.isNaN.) MozReview-Commit-ID: JmlRoidARVp
devtools/client/framework/menu.js
devtools/client/framework/test/browser_toolbox_zoom_popup.js
devtools/client/shared/widgets/tooltip/HTMLTooltip.js
--- a/devtools/client/framework/menu.js
+++ b/devtools/client/framework/menu.js
@@ -1,18 +1,18 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-const Services = require("Services");
 const EventEmitter = require("devtools/shared/event-emitter");
+const { getCurrentZoom } = require("devtools/shared/layout/utils");
 
 /**
  * A partial implementation of the Menu API provided by electron:
  * https://github.com/electron/electron/blob/master/docs/api/menu.md.
  *
  * Extra features:
  *  - Emits an 'open' and 'close' event when the menu is opened/closed
 
@@ -57,20 +57,17 @@ Menu.prototype.insert = function(pos, me
  * element's position to zoom value.
  * If you know the screen coodinate of display position, you should use Menu.pop().
  *
  * @param {int} x
  * @param {int} y
  * @param Toolbox toolbox
  */
 Menu.prototype.popupWithZoom = function(x, y, toolbox) {
-  let zoom = parseFloat(Services.prefs.getCharPref("devtools.toolbox.zoomValue"));
-  if (!zoom || isNaN(zoom)) {
-    zoom = 1.0;
-  }
+  const zoom = getCurrentZoom(toolbox.doc);
   this.popup(x * zoom, y * zoom, toolbox);
 };
 
 /**
  * Show the Menu at a specified location on the screen
  *
  * Missing features:
  *   - browserWindow - BrowserWindow (optional) - Default is null.
--- a/devtools/client/framework/test/browser_toolbox_zoom_popup.js
+++ b/devtools/client/framework/test/browser_toolbox_zoom_popup.js
@@ -9,35 +9,35 @@ const {Toolbox} = require("devtools/clie
 
 // Use simple URL in order to prevent displacing the left positon of frames menu.
 const TEST_URL = "data:text/html;charset=utf-8,<iframe/>";
 
 add_task(async function() {
   registerCleanupFunction(async function() {
     Services.prefs.clearUserPref("devtools.toolbox.zoomValue");
   });
-  const zoom = 1.5;
+  const zoom = 1.4;
   Services.prefs.setCharPref("devtools.toolbox.zoomValue", zoom.toString(10));
 
-  info("Load iframe page for checking the frame menu with x1.5 zoom.");
+  info("Load iframe page for checking the frame menu with x1.4 zoom.");
   await addTab(TEST_URL);
   const target = TargetFactory.forTab(gBrowser.selectedTab);
   const toolbox = await gDevTools.showToolbox(target,
                                             "inspector",
                                             Toolbox.HostType.WINDOW);
   const inspector = toolbox.getCurrentPanel();
   const hostWindow = toolbox.win.parent;
   const originWidth = hostWindow.outerWidth;
   const originHeight = hostWindow.outerHeight;
   const windowUtils = toolbox.win.QueryInterface(Ci.nsIInterfaceRequestor)
       .getInterface(Ci.nsIDOMWindowUtils);
 
-  info("Waiting for the toolbox window will to be rendered with zoom x1.5");
+  info("Waiting for the toolbox window will to be rendered with zoom x1.4");
   await waitUntil(() => {
-    return parseFloat(windowUtils.fullZoom.toFixed(1)) === parseFloat(zoom);
+    return parseFloat(windowUtils.fullZoom.toFixed(1)) === zoom;
   });
 
   info("Resizing and moving the toolbox window in order to display the chevron menu.");
   // If window is displayed bottom of screen, menu might be displayed above of button.
   hostWindow.moveTo(0, 0);
 
   // This size will display inspector's tabs menu button and chevron menu button of toolbox.
   const prevTabs = toolbox.doc.querySelectorAll(".devtools-tab").length;
--- a/devtools/client/shared/widgets/tooltip/HTMLTooltip.js
+++ b/devtools/client/shared/widgets/tooltip/HTMLTooltip.js
@@ -3,18 +3,18 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const EventEmitter = require("devtools/shared/event-emitter");
 const {TooltipToggle} = require("devtools/client/shared/widgets/tooltip/TooltipToggle");
+const {getCurrentZoom} = require("devtools/shared/layout/utils");
 const {listenOnce} = require("devtools/shared/async-utils");
-const Services = require("Services");
 
 const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 const XHTML_NS = "http://www.w3.org/1999/xhtml";
 
 const POSITION = {
   TOP: "top",
   BOTTOM: "bottom",
 };
@@ -615,20 +615,17 @@ HTMLTooltip.prototype = {
     panel.setAttribute("class", "tooltip-xul-wrapper");
 
     return panel;
   },
 
   _showXulWrapperAt: function(left, top) {
     this.xulPanelWrapper.addEventListener("popuphidden", this._onXulPanelHidden);
     const onPanelShown = listenOnce(this.xulPanelWrapper, "popupshown");
-    let zoom = parseFloat(Services.prefs.getCharPref("devtools.toolbox.zoomValue"));
-    if (!zoom || isNaN(zoom)) {
-      zoom = 1.0;
-    }
+    const zoom = getCurrentZoom(this.xulPanelWrapper);
     this.xulPanelWrapper.openPopupAtScreen(left * zoom, top * zoom, false);
     return onPanelShown;
   },
 
   _hideXulWrapper: function() {
     this.xulPanelWrapper.removeEventListener("popuphidden", this._onXulPanelHidden);
 
     if (this.xulPanelWrapper.state === "closed") {