Backed out changeset 09b2d1c8bba2 (bug 1689951) for causing dt failures in browser_screenshot_button_warning. CLOSED TREE
authorsmolnar <smolnar@mozilla.com>
Wed, 10 Feb 2021 16:17:07 +0200
changeset 566812 569826c0fd47a0e196057500c0873fab31253b70
parent 566811 86cdeb4f7d76bdd99626ea3f55e5ce95bb4ee03c
child 566813 5b31293f0b8dc1c58f981b6dd44ceee6c5688736
push id38190
push userbtara@mozilla.com
push dateWed, 10 Feb 2021 21:50:51 +0000
treeherdermozilla-central@569826c0fd47 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1689951
milestone87.0a1
backs out09b2d1c8bba2075330790cf05b641fce84712870
first release with
nightly linux32
569826c0fd47 / 87.0a1 / 20210210215051 / files
nightly linux64
569826c0fd47 / 87.0a1 / 20210210215051 / files
nightly mac
569826c0fd47 / 87.0a1 / 20210210215051 / files
nightly win32
569826c0fd47 / 87.0a1 / 20210210215051 / files
nightly win64
569826c0fd47 / 87.0a1 / 20210210215051 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 09b2d1c8bba2 (bug 1689951) for causing dt failures in browser_screenshot_button_warning. CLOSED TREE
devtools/client/definitions.js
devtools/client/framework/test/browser.ini
devtools/client/framework/test/browser_toolbox_screenshot_tool.js
devtools/client/inspector/inspector.js
devtools/client/inspector/markup/test/browser.ini
devtools/client/inspector/markup/test/browser_markup_screenshot_node_warning.js
devtools/client/responsive/test/browser/browser.ini
devtools/client/responsive/test/browser/browser_screenshot_button.js
devtools/client/responsive/test/browser/browser_screenshot_button_warning.js
devtools/client/responsive/ui.js
devtools/client/shared/test/shared-head.js
--- a/devtools/client/definitions.js
+++ b/devtools/client/definitions.js
@@ -561,46 +561,17 @@ exports.ToolboxButtons = [
       const clipboardEnabled = Services.prefs.getBoolPref(
         "devtools.screenshot.clipboard.enabled"
       );
       const args = { fullpage: true, file: true };
       if (clipboardEnabled) {
         args.clipboard = true;
       }
 
-      const messages = await captureAndSaveScreenshot(
-        toolbox.target,
-        toolbox.win,
-        args
-      );
-      const notificationBox = toolbox.getNotificationBox();
-      const priorityMap = {
-        error: notificationBox.PRIORITY_CRITICAL_HIGH,
-        warn: notificationBox.PRIORITY_WARNING_HIGH,
-      };
-      for (const { text, level } of messages) {
-        // captureAndSaveScreenshot returns "saved" messages, that indicate where the
-        // screenshot was saved. In regular toolbox, we don't want to display them as
-        // the download UI can be used to open them.
-        // But in the browser toolbox, we can't see the download UI, so we'll display the
-        // saved message so the user knows there the file was saved.
-        if (
-          !toolbox.target.isParentProcess &&
-          level !== "warn" &&
-          level !== "error"
-        ) {
-          continue;
-        }
-        notificationBox.appendNotification(
-          text,
-          null,
-          null,
-          priorityMap[level] || notificationBox.PRIORITY_INFO_MEDIUM
-        );
-      }
+      await captureAndSaveScreenshot(toolbox.target, toolbox.win, args);
     },
   },
   createHighlightButton("RulersHighlighter", "rulers"),
   createHighlightButton("MeasuringToolHighlighter", "measure"),
 ];
 
 function createHighlightButton(highlighterName, id) {
   return {
--- a/devtools/client/framework/test/browser.ini
+++ b/devtools/client/framework/test/browser.ini
@@ -113,17 +113,16 @@ skip-if = fission # Disable frequent fis
 [browser_toolbox_options_frames_button.js]
 [browser_toolbox_options_panel_toggle.js]
 [browser_toolbox_raise.js]
 disabled=Bug 962258
 [browser_toolbox_races.js]
 [browser_toolbox_ready.js]
 [browser_toolbox_remoteness_change.js]
 run-if = e10s
-[browser_toolbox_screenshot_tool.js]
 [browser_toolbox_select_event.js]
 [browser_toolbox_selected_tool_unavailable.js]
 [browser_toolbox_selectionchanged_event.js]
 [browser_toolbox_show_toolbox_tool_ready.js]
 [browser_toolbox_split_console.js]
 [browser_toolbox_target.js]
 [browser_toolbox_tabsswitch_shortcuts.js]
 [browser_toolbox_telemetry_activate_splitconsole.js]
deleted file mode 100644
--- a/devtools/client/framework/test/browser_toolbox_screenshot_tool.js
+++ /dev/null
@@ -1,85 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-
-const exampleNetDocument = `http://example.net/document-builder.sjs`;
-const exampleComDocument = `http://example.com/document-builder.sjs`;
-
-const TEST_URL = `${exampleNetDocument}?html=
-  <style>
-    body {
-      margin: 0;
-      height: 10001px;
-    }
-    iframe {
-      height: 50px;
-      border:none;
-      display: block;
-    }
-  </style>
-  <iframe
-    src="${exampleNetDocument}?html=<body style='margin:0;height:30px;background:rgb(255,0,0)'></body>"
-    id="same-origin"></iframe>
-  <iframe
-    src="${exampleComDocument}?html=<body style='margin:0;height:30px;background:rgb(0,255,0)'></body>"
-    id="remote"></iframe>`;
-
-add_task(async function() {
-  await pushPref("devtools.command-button-screenshot.enabled", true);
-
-  await addTab(TEST_URL);
-  const target = await TargetFactory.forTab(gBrowser.selectedTab);
-
-  info("Open the toolbox");
-  const toolbox = await gDevTools.showToolbox(target, "console");
-
-  const onScreenshotDownloaded = waitUntilScreenshot();
-  toolbox.doc.querySelector("#command-button-screenshot").click();
-  const filePath = await onScreenshotDownloaded;
-
-  ok(filePath, "The screenshot was taken");
-
-  info("Create an image using the downloaded file as source");
-  const image = new Image();
-  const onImageLoad = once(image, "load");
-  image.src = OS.Path.toFileURI(filePath);
-  await onImageLoad;
-
-  const dpr = await SpecialPowers.spawn(
-    gBrowser.selectedBrowser,
-    [],
-    () => content.wrappedJSObject.devicePixelRatio
-  );
-
-  info("Check that the same-origin iframe is rendered in the screenshot");
-  await checkImageColorAt({
-    image,
-    y: 10 * dpr,
-    expectedColor: `rgb(255, 0, 0)`,
-    label: "The same-origin iframe is rendered properly in the screenshot",
-  });
-
-  info("Check that the remote iframe is rendered in the screenshot");
-  await checkImageColorAt({
-    image,
-    y: 60 * dpr,
-    expectedColor: `rgb(0, 255, 0)`,
-    label: "The remote iframe is rendered properly in the screenshot",
-  });
-
-  info(
-    "Check that a warning message was displayed to indicate the screenshot was truncated"
-  );
-  const notificationBox = await waitFor(() =>
-    toolbox.doc.querySelector(".notificationbox")
-  );
-
-  const message = notificationBox.querySelector(".notification").textContent;
-  ok(
-    message.startsWith("The image was cut off"),
-    `The warning message is rendered as expected (${message})`
-  );
-
-  //Remove the downloaded screenshot file
-  await OS.File.remove(filePath);
-  await resetDownloads();
-});
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -1904,35 +1904,21 @@ Inspector.prototype = {
       "devtools.screenshot.clipboard.enabled"
     );
     const args = {
       file: true,
       nodeActorID: this.selection.nodeFront.actorID,
       clipboard: clipboardEnabled,
     };
 
-    const messages = await captureAndSaveScreenshot(
+    await captureAndSaveScreenshot(
       this.selection.nodeFront.targetFront,
       this.panelWin,
       args
     );
-    const notificationBox = this.toolbox.getNotificationBox();
-    const priorityMap = {
-      error: notificationBox.PRIORITY_CRITICAL_HIGH,
-      warn: notificationBox.PRIORITY_WARNING_HIGH,
-    };
-    for (const { text, level } of messages) {
-      // captureAndSaveScreenshot returns "saved" messages, that indicate where the
-      // screenshot was saved. We don't want to display them as the download UI can be
-      // used to open the file.
-      if (level !== "warn" && level !== "error") {
-        continue;
-      }
-      notificationBox.appendNotification(text, null, null, priorityMap[level]);
-    }
   },
 
   /**
    * Returns an object containing the shared handler functions used in React components.
    */
   getCommonComponentProps() {
     return {
       setSelectedNode: this.selection.setNodeFront,
--- a/devtools/client/inspector/markup/test/browser.ini
+++ b/devtools/client/inspector/markup/test/browser.ini
@@ -186,17 +186,16 @@ skip-if = verify
 [browser_markup_pagesize_01.js]
 [browser_markup_pagesize_02.js]
 [browser_markup_pseudo_on_reload.js]
 [browser_markup_remove_xul_attributes.js]
 [browser_markup_screenshot_node.js]
 [browser_markup_screenshot_node_about_page.js]
 [browser_markup_screenshot_node_iframe.js]
 [browser_markup_screenshot_node_shadowdom.js]
-[browser_markup_screenshot_node_warning.js]
 [browser_markup_scrollable_badge.js]
 [browser_markup_scrollable_badge_click.js]
 [browser_markup_search_01.js]
 [browser_markup_shadowdom.js]
 [browser_markup_shadowdom_clickreveal.js]
 [browser_markup_shadowdom_clickreveal_scroll.js]
 [browser_markup_shadowdom_copy_paths.js]
 [browser_markup_shadowdom_delete.js]
deleted file mode 100644
--- a/devtools/client/inspector/markup/test/browser_markup_screenshot_node_warning.js
+++ /dev/null
@@ -1,38 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-const TEST_URL = `data:text/html;charset=utf8,
-  <div id="blue-node" style="width:30px;height:11000px;background:rgb(0, 0, 255)"></div>`;
-
-// Test taking a screenshot of a tall node displays a warning message in the notification box.
-add_task(async function() {
-  const { inspector, toolbox } = await openInspectorForURL(encodeURI(TEST_URL));
-
-  info("Select the blue node");
-  await selectNode("#blue-node", inspector);
-
-  info("Take a screenshot of the blue node and verify it looks as expected");
-  const blueScreenshot = await takeNodeScreenshot(inspector);
-  await assertSingleColorScreenshotImage(blueScreenshot, 30, 10000, {
-    r: 0,
-    g: 0,
-    b: 255,
-  });
-
-  info(
-    "Check that a warning message was displayed to indicate the screenshot was truncated"
-  );
-  const notificationBox = await waitFor(() =>
-    toolbox.doc.querySelector(".notificationbox")
-  );
-
-  const message = notificationBox.querySelector(".notification").textContent;
-  ok(
-    message.startsWith("The image was cut off"),
-    `The warning message is rendered as expected (${message})`
-  );
-
-  await toolbox.destroy();
-});
--- a/devtools/client/responsive/test/browser/browser.ini
+++ b/devtools/client/responsive/test/browser/browser.ini
@@ -58,17 +58,16 @@ tags = devtools webextensions
 [browser_page_redirection.js]
 skip-if = os == "linux"
 [browser_page_state.js]
 [browser_page_style.js]
 [browser_permission_doorhanger.js]
 tags = devtools geolocation
 [browser_picker_link.js]
 [browser_preloaded_newtab.js]
-[browser_screenshot_button_warning.js]
 [browser_screenshot_button.js]
 [browser_scroll.js]
 [browser_state_restore.js]
 [browser_tab_close.js]
 [browser_tab_not_selected.js]
 [browser_tab_remoteness_change.js]
 [browser_tab_remoteness_change_fission_switch_target.js]
 [browser_target_blank.js]
--- a/devtools/client/responsive/test/browser/browser_screenshot_button.js
+++ b/devtools/client/responsive/test/browser/browser_screenshot_button.js
@@ -1,14 +1,14 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-// Test global screenshot button
+// Test global exit button
 
 const TEST_URL = "data:text/html;charset=utf-8,";
 
 const { OS } = require("resource://gre/modules/osfile.jsm");
 
 async function waitUntilScreenshot() {
   const { Downloads } = require("resource://gre/modules/Downloads.jsm");
   const list = await Downloads.getList(Downloads.ALL);
deleted file mode 100644
--- a/devtools/client/responsive/test/browser/browser_screenshot_button_warning.js
+++ /dev/null
@@ -1,107 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-// Test that warning messages emitted when taking a screenshot are displayed in the UI.
-
-const exampleNetDocument = `http://example.net/document-builder.sjs`;
-const exampleComDocument = `http://example.com/document-builder.sjs`;
-
-const TEST_URL = `${exampleNetDocument}?html=
-  <style>
-    body {
-      margin: 0;
-      height: 10001px;
-    }
-    iframe {
-      height: 50px;
-      border:none;
-      display: block;
-    }
-  </style>
-  <iframe
-    src="${exampleNetDocument}?html=<body style='margin:0;height:30px;background:rgb(255,0,0)'></body>"
-    id="same-origin" onload="document.body.classList.add('same-origin-loaded')"></iframe>
-  <iframe
-    src="${exampleComDocument}?html=<body style='margin:0;height:30px;background:rgb(0,255,0)'></body>"
-    id="remote" onload="document.body.classList.add('remote-origin-loaded')"></iframe>`;
-
-addRDMTask(
-  TEST_URL,
-  async function({ ui, browser, manager }) {
-    const { toolWindow } = ui;
-    const { document } = toolWindow;
-
-    info("Wait until the iframes are loaded");
-    await waitUntil(() =>
-      SpecialPowers.spawn(ui.getViewportBrowser(), [], async function() {
-        const { classList } = content.document.body;
-        return (
-          classList.contains("same-origin-loaded") &&
-          classList.contains("remote-loaded")
-        );
-      })
-    );
-
-    info(
-      "Set a big viewport and high dpr so the screenshot dpr gets downsized"
-    );
-    // The viewport can't be bigger than 9999×9999
-    await setViewportSize(ui, manager, 9999, 9999);
-    const dpr = 3;
-    await selectDevicePixelRatio(ui, dpr);
-    await waitForDevicePixelRatio(ui, dpr);
-    // Wait for a tick so the page can be re-rendered.
-    await waitForTick();
-
-    info("Click the screenshot button");
-    const onScreenshotDownloaded = waitUntilScreenshot();
-    const screenshotButton = document.getElementById("screenshot-button");
-    screenshotButton.click();
-
-    const filePath = await onScreenshotDownloaded;
-    ok(filePath, "The screenshot was taken");
-
-    const image = new Image();
-    const onImageLoad = once(image, "load");
-    image.src = OS.Path.toFileURI(filePath);
-    await onImageLoad;
-
-    info("Check that the same-origin iframe is rendered in the screenshot");
-    await checkImageColorAt({
-      image,
-      y: 10,
-      expectedColor: `rgb(255, 0, 0)`,
-      label: "The same-origin iframe is rendered properly in the screenshot",
-    });
-
-    info("Check that the remote iframe is rendered in the screenshot");
-    await checkImageColorAt({
-      image,
-      y: 60,
-      expectedColor: `rgb(0, 255, 0)`,
-      label: "The remote iframe is rendered properly in the screenshot",
-    });
-
-    info(
-      "Check that a warning message was displayed to indicate the dpr was changed"
-    );
-
-    const box = gBrowser.getNotificationBox(browser);
-    await waitUntil(() => box.currentNotification);
-
-    const notificationEl = box.currentNotification;
-    ok(notificationEl, "Notification should be visible");
-    is(
-      notificationEl.textContent,
-      "The device pixel ratio was reduced to 1 as the resulting image was too large",
-      "The expected warning was displayed"
-    );
-
-    //Remove the downloaded screenshot file
-    await OS.File.remove(filePath);
-    await resetDownloads();
-  },
-  { waitForDeviceList: true }
-);
--- a/devtools/client/responsive/ui.js
+++ b/devtools/client/responsive/ui.js
@@ -39,22 +39,16 @@ loader.lazyRequireGetter(
   "devtools/client/responsive/utils/message"
 );
 loader.lazyRequireGetter(
   this,
   "showNotification",
   "devtools/client/responsive/utils/notification",
   true
 );
-loader.lazyRequireGetter(
-  this,
-  "PriorityLevels",
-  "devtools/client/shared/components/NotificationBox",
-  true
-);
 loader.lazyRequireGetter(this, "l10n", "devtools/client/responsive/utils/l10n");
 loader.lazyRequireGetter(this, "asyncStorage", "devtools/shared/async-storage");
 loader.lazyRequireGetter(
   this,
   "captureAndSaveScreenshot",
   "devtools/client/shared/screenshot",
   true
 );
@@ -699,39 +693,17 @@ class ResponsiveUI {
   }
 
   async onRotateViewport(event) {
     const { orientationType: type, angle, isViewportRotated } = event.data;
     await this.updateScreenOrientation(type, angle, isViewportRotated);
   }
 
   async onScreenshot() {
-    const messages = await captureAndSaveScreenshot(
-      this.currentTarget,
-      this.browserWindow
-    );
-
-    const priorityMap = {
-      error: PriorityLevels.PRIORITY_CRITICAL_HIGH,
-      warn: PriorityLevels.PRIORITY_WARNING_HIGH,
-    };
-    for (const { text, level } of messages) {
-      // captureAndSaveScreenshot returns "saved" messages, that indicate where the
-      // screenshot was saved. We don't want to display them as the download UI can be
-      // used to open the file.
-      if (level !== "warn" && level !== "error") {
-        continue;
-      }
-
-      showNotification(this.browserWindow, this.tab, {
-        msg: text,
-        priority: priorityMap[level],
-      });
-    }
-
+    await captureAndSaveScreenshot(this.currentTarget, this.browserWindow);
     message.post(this.rdmFrame.contentWindow, "screenshot-captured");
   }
 
   onToggleLeftAlignment(event) {
     this.updateUIAlignment(event.data.leftAlignmentEnabled);
   }
 
   onUpdateDeviceModal(event) {
--- a/devtools/client/shared/test/shared-head.js
+++ b/devtools/client/shared/test/shared-head.js
@@ -1278,19 +1278,18 @@ async function takeNodeScreenshot(inspec
     "Call screenshotNode() and wait until the screenshot is found in the Downloads"
   );
   const whenScreenshotSucceeded = waitUntilScreenshot();
   inspector.screenshotNode();
   const filePath = await whenScreenshotSucceeded;
 
   info("Create an image using the downloaded fileas source");
   const image = new Image();
-  const onImageLoad = once(image, "load");
   image.src = OS.Path.toFileURI(filePath);
-  await onImageLoad;
+  await once(image, "load");
 
   info("Remove the downloaded screenshot file");
   await OS.File.remove(filePath);
 
   // See intermittent Bug 1508435. Even after removing the file, tests still manage to
   // reuse files from the previous test if they have the same name. Since our file name
   // is based on a timestamp that has "second" precision, wait for one second to make sure
   // screenshots will have different names.