Backed out changeset 4726abbc0c2c (bug 1874026) for causing multiple …

…frequent bc failures. CLOSED TREE
dothq · Feb 2, 2024 · 195daa2 · 195daa2
1 parent f5f3450
commit 195daa2
Show file tree

Hide file tree

Showing 10 changed files with 151 additions and 340 deletions.
diff --git a/browser/components/screenshots/ScreenshotsOverlayChild.sys.mjs b/browser/components/screenshots/ScreenshotsOverlayChild.sys.mjs
@@ -203,7 +203,7 @@ export class ScreenshotsOverlay {
     this.#content.root.appendChild(this.fragment);
 
     this.initializeElements();
-    await this.updateWindowDimensions();
+    this.updateWindowDimensions();
 
     this.#setState(STATES.CROSSHAIRS);
 
@@ -1451,23 +1451,20 @@ export class ScreenshotsOverlay {
    * container size so we don't draw outside the page bounds.
    * @param {String} eventType will be "scroll" or "resize"
    */
-  async updateScreenshotsOverlayDimensions(eventType) {
-    let updateWindowDimensionsPromise = this.updateWindowDimensions();
+  updateScreenshotsOverlayDimensions(eventType) {
+    this.updateWindowDimensions();
 
     if (this.#state === STATES.CROSSHAIRS) {
       if (eventType === "resize") {
         this.hideHoverElementContainer();
-        this.#cachedEle = null;
+        this.updatePreviewContainer();
       } else if (eventType === "scroll") {
         if (this.#lastClientX && this.#lastClientY) {
           this.#cachedEle = null;
           this.handleElementHover(this.#lastClientX, this.#lastClientY);
         }
       }
     } else if (this.#state === STATES.SELECTED) {
-      await updateWindowDimensionsPromise;
-      this.selectionRegion.shift();
-      this.drawSelectionContainer();
       this.drawButtonsContainer();
       this.updateSelectionSizeText();
     }
@@ -1530,21 +1527,13 @@ export class ScreenshotsOverlay {
   }
 
   /**
-   * We have to be careful not to draw the overlay larger than the document
-   * because the overlay is absolutely position and within the document so we
-   * can cause the document to overflow when it shouldn't. To mitigate this,
-   * we will temporarily position the overlay to position fixed with width and
-   * height 100% so the overlay is within the document bounds. Then we will get
-   * the dimensions of the document to correctly draw the overlay.
+   * Update the screenshots container because the window has changed size of
+   * scrolled. The screenshots-overlay-container doesn't shrink with the page
+   * when the window is resized so we have to manually find the width and
+   * height of the page and make sure we aren't drawing outside the actual page
+   * dimensions.
    */
-  async updateWindowDimensions() {
-    // Setting the screenshots container attribute "resizing" will make the
-    // overlay fixed position with width and height of 100% percent so it
-    // does not draw outside the actual document.
-    this.screenshotsContainer.toggleAttribute("resizing", true);
-
-    await new Promise(r => this.window.requestAnimationFrame(r));
-
+  updateWindowDimensions() {
     let {
       clientWidth,
       clientHeight,
@@ -1555,7 +1544,49 @@ export class ScreenshotsOverlay {
       scrollMinX,
       scrollMinY,
     } = this.getDimensionsFromWindow();
-    this.screenshotsContainer.toggleAttribute("resizing", false);
+
+    let shouldUpdate = true;
+
+    if (
+      clientHeight < this.windowDimensions.clientHeight ||
+      clientWidth < this.windowDimensions.clientWidth
+    ) {
+      let widthDiff = this.windowDimensions.clientWidth - clientWidth;
+      let heightDiff = this.windowDimensions.clientHeight - clientHeight;
+
+      this.windowDimensions.dimensions = {
+        scrollWidth: scrollWidth - Math.max(widthDiff, 0),
+        scrollHeight: scrollHeight - Math.max(heightDiff, 0),
+        clientWidth,
+        clientHeight,
+      };
+
+      if (this.#state === STATES.SELECTED) {
+        let didShift = this.selectionRegion.shift();
+        if (didShift) {
+          this.drawSelectionContainer();
+          this.drawButtonsContainer();
+        }
+      } else if (this.#state === STATES.CROSSHAIRS) {
+        this.updatePreviewContainer();
+      }
+      this.updateScreenshotsOverlayContainer();
+      // We just updated the screenshots container so we check if the window
+      // dimensions are still accurate
+      let { scrollWidth: updatedWidth, scrollHeight: updatedHeight } =
+        this.getDimensionsFromWindow();
+
+      // If the width and height are the same then we don't need to draw the overlay again
+      if (updatedWidth === scrollWidth && updatedHeight === scrollHeight) {
+        shouldUpdate = false;
+      }
+
+      scrollWidth = updatedWidth;
+      scrollHeight = updatedHeight;
+    }
+
+    setMaxDetectHeight(Math.max(clientHeight + 100, 700));
+    setMaxDetectWidth(Math.max(clientWidth + 100, 1000));
 
     this.windowDimensions.dimensions = {
       clientWidth,
@@ -1569,10 +1600,9 @@ export class ScreenshotsOverlay {
       devicePixelRatio: this.window.devicePixelRatio,
     };
 
-    this.updatePreviewContainer();
-    this.updateScreenshotsOverlayContainer();
-
-    setMaxDetectHeight(Math.max(clientHeight + 100, 700));
-    setMaxDetectWidth(Math.max(clientWidth + 100, 1000));
+    if (shouldUpdate) {
+      this.updatePreviewContainer();
+      this.updateScreenshotsOverlayContainer();
+    }
   }
 }
diff --git a/browser/components/screenshots/overlay/overlay.css b/browser/components/screenshots/overlay/overlay.css
@@ -24,12 +24,6 @@
   &[dragging] {
     cursor: grabbing;
   }
-
-  &[resizing] {
-    position: fixed;
-    width: 100% !important;
-    height: 100% !important;
-  }
 }
 
 #selection-container {

diff --git a/browser/components/screenshots/overlayHelpers.mjs b/browser/components/screenshots/overlayHelpers.mjs
@@ -248,16 +248,18 @@ export class Region {
     let didShift = false;
     let xDiff = this.right - this.#windowDimensions.scrollWidth;
     if (xDiff > 0) {
-      this.left -= xDiff;
       this.right -= xDiff;
+      this.left -= xDiff;
 
       didShift = true;
     }
 
     let yDiff = this.bottom - this.#windowDimensions.scrollHeight;
     if (yDiff > 0) {
-      this.top -= yDiff;
+      let curHeight = this.height;
+
       this.bottom -= yDiff;
+      this.top = this.bottom - curHeight;
 
       didShift = true;
     }

diff --git a/browser/components/screenshots/tests/browser/browser.toml b/browser/components/screenshots/tests/browser/browser.toml
@@ -4,7 +4,6 @@ support-files = [
   "test-page.html",
   "short-test-page.html",
   "large-test-page.html",
-  "test-page-resize.html",
 ]
 
 prefs = [
@@ -46,5 +45,3 @@ skip-if = ["!crashreporter"]
 ["browser_screenshots_test_visible.js"]
 
 ["browser_test_element_picker.js"]
-
-["browser_test_resize.js"]
diff --git a/browser/components/screenshots/tests/browser/browser_screenshots_drag_scroll_test.js b/browser/components/screenshots/tests/browser/browser_screenshots_drag_scroll_test.js
@@ -227,7 +227,7 @@ add_task(async function test_scrollingScreenshotsOpen() {
       is(dimensions.height, endY - startY, "The box height is now 90");
 
       // reset screenshots box
-      await helper.escapeKeyInContent();
+      mouse.click(scrollX + startX, scrollY + endY);
       await helper.assertStateChange("crosshairs");
 
       await helper.dragOverlay(
@@ -247,7 +247,7 @@ add_task(async function test_scrollingScreenshotsOpen() {
       is(dimensions.height, endY - startY, "The box height is now 90");
 
       // reset screenshots box
-      await helper.escapeKeyInContent();
+      mouse.click(10, 10);
       await helper.assertStateChange("crosshairs");
 
       await helper.dragOverlay(
@@ -380,7 +380,11 @@ add_task(async function test_scrollIfByEdge() {
           contentInfo
         )}\n`
       );
-      await helper.dragOverlay(startX, startY, endX, endY, "selected");
+      mouse.down(startX, startY);
+      await helper.assertStateChange("resizing");
+      mouse.move(endX, endY);
+      mouse.up(endX, endY);
+      await helper.assertStateChange("selected");
 
       windowX = 1000;
       windowY = 1000;
@@ -419,12 +423,20 @@ add_task(async function test_scrollIfByEdgeWithKeyboard() {
 
       await helper.dragOverlay(1020, 1020, 1120, 1120);
 
-      await helper.moveOverlayViaKeyboard("highlight", [
-        { key: "ArrowLeft", options: { shiftKey: true } },
-        { key: "ArrowLeft", options: {} },
-        { key: "ArrowUp", options: { shiftKey: true } },
-        { key: "ArrowUp", options: {} },
-      ]);
+      await SpecialPowers.spawn(browser, [], async () => {
+        let screenshotsChild = content.windowGlobalChild.getActor(
+          "ScreenshotsComponent"
+        );
+
+        // Test moving each corner of the region
+        screenshotsChild.overlay.highlightEl.focus();
+
+        EventUtils.synthesizeKey("ArrowLeft", { shiftKey: true }, content);
+        EventUtils.synthesizeKey("ArrowLeft", {}, content);
+
+        EventUtils.synthesizeKey("ArrowUp", { shiftKey: true }, content);
+        EventUtils.synthesizeKey("ArrowUp", {}, content);
+      });
 
       windowX = 989;
       windowY = 989;
@@ -445,12 +457,20 @@ add_task(async function test_scrollIfByEdgeWithKeyboard() {
         scrollY + clientHeight - 20
       );
 
-      await helper.moveOverlayViaKeyboard("highlight", [
-        { key: "ArrowRight", options: { shiftKey: true } },
-        { key: "ArrowRight", options: {} },
-        { key: "ArrowDown", options: { shiftKey: true } },
-        { key: "ArrowDown", options: {} },
-      ]);
+      await SpecialPowers.spawn(browser, [], async () => {
+        let screenshotsChild = content.windowGlobalChild.getActor(
+          "ScreenshotsComponent"
+        );
+
+        // Test moving each corner of the region
+        screenshotsChild.overlay.highlightEl.focus();
+
+        EventUtils.synthesizeKey("ArrowRight", { shiftKey: true }, content);
+        EventUtils.synthesizeKey("ArrowRight", {}, content);
+
+        EventUtils.synthesizeKey("ArrowDown", { shiftKey: true }, content);
+        EventUtils.synthesizeKey("ArrowDown", {}, content);
+      });
 
       windowX = 1000;
       windowY = 1000;

diff --git a/browser/components/screenshots/tests/browser/browser_screenshots_short_page_test.js b/browser/components/screenshots/tests/browser/browser_screenshots_short_page_test.js
@@ -58,7 +58,20 @@ add_task(async function test_window_resize() {
       const BIG_WINDOW_SIZE = 920;
       const SMALL_WINDOW_SIZE = 620;
 
-      await helper.resizeContentWindow(SMALL_WINDOW_SIZE, SMALL_WINDOW_SIZE);
+      window.resizeTo(SMALL_WINDOW_SIZE, SMALL_WINDOW_SIZE);
+      await TestUtils.waitForCondition(() => {
+        info(
+          `Got ${window.outerWidth}x${
+            window.outerHeight
+          }. Expecting ${SMALL_WINDOW_SIZE}. ${
+            window.outerHeight === SMALL_WINDOW_SIZE
+          } ${window.outerWidth === SMALL_WINDOW_SIZE}`
+        );
+        return (
+          window.outerHeight === SMALL_WINDOW_SIZE &&
+          window.outerWidth === SMALL_WINDOW_SIZE
+        );
+      }, "Waiting for window to resize");
 
       helper.triggerUIFromToolbar();
       await helper.waitForOverlay();
@@ -68,7 +81,13 @@ add_task(async function test_window_resize() {
       let oldWidth = dimensions.scrollWidth;
       let oldHeight = dimensions.scrollHeight;
 
-      await helper.resizeContentWindow(BIG_WINDOW_SIZE, BIG_WINDOW_SIZE);
+      window.resizeTo(BIG_WINDOW_SIZE, BIG_WINDOW_SIZE);
+      await TestUtils.waitForCondition(
+        () =>
+          window.outerHeight === BIG_WINDOW_SIZE &&
+          window.outerWidth === BIG_WINDOW_SIZE,
+        "Waiting for window to resize"
+      );
       await helper.waitForSelectionLayerDimensionChange(oldWidth, oldHeight);
 
       contentInfo = await helper.getContentDimensions();
@@ -87,7 +106,13 @@ add_task(async function test_window_resize() {
       oldWidth = dimensions.scrollWidth;
       oldHeight = dimensions.scrollHeight;
 
-      await helper.resizeContentWindow(SMALL_WINDOW_SIZE, SMALL_WINDOW_SIZE);
+      window.resizeTo(SMALL_WINDOW_SIZE, SMALL_WINDOW_SIZE);
+      await TestUtils.waitForCondition(
+        () =>
+          window.outerHeight === SMALL_WINDOW_SIZE &&
+          window.outerWidth === SMALL_WINDOW_SIZE,
+        "Waiting for window to resize"
+      );
       await helper.waitForSelectionLayerDimensionChange(oldWidth, oldHeight);
 
       contentInfo = await helper.getContentDimensions();
@@ -114,9 +139,12 @@ add_task(async function test_window_resize() {
         "Screenshots overlay is smaller than the big window height"
       );
 
-      await helper.resizeContentWindow(
-        originalWindowWidth,
-        originalWindowHeight
+      window.resizeTo(originalWindowWidth, originalWindowHeight);
+      await TestUtils.waitForCondition(
+        () =>
+          window.outerHeight === originalWindowHeight &&
+          window.outerWidth === originalWindowWidth,
+        "Waiting for window to resize"
       );
     }
   );

diff --git a/browser/components/screenshots/tests/browser/browser_screenshots_test_toggle_pref.js b/browser/components/screenshots/tests/browser/browser_screenshots_test_toggle_pref.js
@@ -284,6 +284,4 @@ add_task(async function test() {
 
   observerStub.restore();
   notifierStub.restore();
-
-  await SpecialPowers.popPrefEnv();
 });