Bug 1404917 - Move image preview tooltip to file name and remove preview icon. r=Honza
☠☠ backed out by 004b34f82c44 ☠ ☠
authorAlexandre Poirot <poirot.alex@gmail.com>
Mon, 06 Nov 2017 06:31:40 -0800
changeset 391287 3e7a6e920c6badbaa1b2e08c748d771583e2a47e
parent 391286 7dcfe8d12d6f4914d2eba8e5dcaab5660d3c6e61
child 391288 8acc12cf30b7c311d22bb60a03cfc3fac94bc8c6
push id97236
push userryanvm@gmail.com
push dateFri, 10 Nov 2017 21:14:41 +0000
treeherdermozilla-inbound@abc17e0eea77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1404917
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1404917 - Move image preview tooltip to file name and remove preview icon. r=Honza MozReview-Commit-ID: 86YPdHZhCmo
devtools/client/netmonitor/src/assets/styles/RequestList.css
devtools/client/netmonitor/src/components/RequestListColumnFile.js
devtools/client/netmonitor/src/components/RequestListContent.js
devtools/client/netmonitor/src/components/RequestListItem.js
devtools/client/netmonitor/src/connector/chrome/response.js
devtools/client/netmonitor/src/connector/firefox-data-provider.js
devtools/client/netmonitor/src/reducers/requests.js
devtools/client/netmonitor/src/request-list-tooltip.js
devtools/client/netmonitor/test/browser.ini
devtools/client/netmonitor/test/browser_net_icon-preview.js
devtools/client/netmonitor/test/browser_net_image-tooltip.js
devtools/client/netmonitor/test/browser_net_thumbnail-click.js
--- a/devtools/client/netmonitor/src/assets/styles/RequestList.css
+++ b/devtools/client/netmonitor/src/assets/styles/RequestList.css
@@ -296,25 +296,16 @@
 .requests-list-file {
   width: 22%;
 }
 
 .requests-list-file.requests-list-column {
   text-align: start;
 }
 
-.requests-list-icon {
-  background: transparent;
-  width: 15px;
-  height: 15px;
-  margin: 0 4px;
-  outline: 1px solid var(--table-splitter-color);
-  vertical-align: top;
-}
-
 /* Protocol column */
 
 .requests-list-protocol {
   width: 8%;
 }
 
 /* Cookies column */
 
--- a/devtools/client/netmonitor/src/components/RequestListColumnFile.js
+++ b/devtools/client/netmonitor/src/components/RequestListColumnFile.js
@@ -6,50 +6,42 @@
 
 const {
   Component,
   DOM,
   PropTypes,
 } = require("devtools/client/shared/vendor/react");
 const { propertiesEqual } = require("../utils/request-utils");
 
-const { div, img } = DOM;
+const { div } = DOM;
 
 const UPDATED_FILE_PROPS = [
-  "responseContentDataUri",
   "urlDetails",
 ];
 
 class RequestListColumnFile extends Component {
   static get propTypes() {
     return {
       item: PropTypes.object.isRequired,
-      onThumbnailMouseDown: PropTypes.func.isRequired,
     };
   }
 
   shouldComponentUpdate(nextProps) {
     return !propertiesEqual(UPDATED_FILE_PROPS, this.props.item, nextProps.item);
   }
 
   render() {
     let {
-      item: { responseContentDataUri, urlDetails },
-      onThumbnailMouseDown
+      item: { urlDetails },
     } = this.props;
 
     return (
       div({
         className: "requests-list-column requests-list-file",
         title: urlDetails.unicodeUrl,
       },
-        img({
-          className: "requests-list-icon",
-          src: responseContentDataUri,
-          onMouseDown: onThumbnailMouseDown,
-        }),
         urlDetails.baseNameWithQuery
       )
     );
   }
 }
 
 module.exports = RequestListColumnFile;
--- a/devtools/client/netmonitor/src/components/RequestListContent.js
+++ b/devtools/client/netmonitor/src/components/RequestListContent.js
@@ -42,17 +42,16 @@ class RequestListContent extends Compone
       dispatch: PropTypes.func.isRequired,
       displayedRequests: PropTypes.object.isRequired,
       firstRequestStartedMillis: PropTypes.number.isRequired,
       fromCache: PropTypes.bool,
       onCauseBadgeMouseDown: PropTypes.func.isRequired,
       onItemMouseDown: PropTypes.func.isRequired,
       onSecurityIconMouseDown: PropTypes.func.isRequired,
       onSelectDelta: PropTypes.func.isRequired,
-      onThumbnailMouseDown: PropTypes.func.isRequired,
       onWaterfallMouseDown: PropTypes.func.isRequired,
       scale: PropTypes.number,
       selectedRequestId: PropTypes.string,
     };
   }
 
   constructor(props) {
     super(props);
@@ -144,17 +143,17 @@ class RequestListContent extends Compone
       return false;
     }
     let requestItem = this.props.displayedRequests.find(r => r.id == itemId);
     if (!requestItem) {
       return false;
     }
 
     let { connector } = this.props;
-    if (requestItem.responseContent && target.closest(".requests-list-icon")) {
+    if (target.closest(".requests-list-file")) {
       return setTooltipImageContent(connector, tooltip, itemEl, requestItem);
     }
 
     return false;
   }
 
   /**
    * Scroll listener for the requests menu view.
@@ -217,17 +216,16 @@ class RequestListContent extends Compone
   render() {
     const {
       columns,
       displayedRequests,
       firstRequestStartedMillis,
       onCauseBadgeMouseDown,
       onItemMouseDown,
       onSecurityIconMouseDown,
-      onThumbnailMouseDown,
       onWaterfallMouseDown,
       scale,
       selectedRequestId,
     } = this.props;
 
     return (
       div({ className: "requests-list-wrapper"},
         div({ className: "requests-list-table"},
@@ -247,17 +245,16 @@ class RequestListContent extends Compone
               index,
               isSelected: item.id === selectedRequestId,
               key: item.id,
               onContextMenu: this.onContextMenu,
               onFocusedNodeChange: this.onFocusedNodeChange,
               onMouseDown: () => onItemMouseDown(item.id),
               onCauseBadgeMouseDown: () => onCauseBadgeMouseDown(item.cause),
               onSecurityIconMouseDown: () => onSecurityIconMouseDown(item.securityState),
-              onThumbnailMouseDown: () => onThumbnailMouseDown(),
               onWaterfallMouseDown: () => onWaterfallMouseDown(),
             }))
           )
         )
       )
     );
   }
 }
@@ -287,22 +284,15 @@ module.exports = connect(
      */
     onSecurityIconMouseDown: (securityState) => {
       if (securityState && securityState !== "insecure") {
         dispatch(Actions.selectDetailsPanelTab("security"));
       }
     },
     onSelectDelta: (delta) => dispatch(Actions.selectDelta(delta)),
     /**
-     * A handler that opens the response tab in the details view if
-     * the thumbnail is clicked.
-     */
-    onThumbnailMouseDown: () => {
-      dispatch(Actions.selectDetailsPanelTab("response"));
-    },
-    /**
      * A handler that opens the timing sidebar panel if the waterfall is clicked.
      */
     onWaterfallMouseDown: () => {
       dispatch(Actions.selectDetailsPanelTab("timings"));
     },
   }),
 )(RequestListContent);
--- a/devtools/client/netmonitor/src/components/RequestListItem.js
+++ b/devtools/client/netmonitor/src/components/RequestListItem.js
@@ -82,17 +82,16 @@ class RequestListItem extends Component 
       isSelected: PropTypes.bool.isRequired,
       firstRequestStartedMillis: PropTypes.number.isRequired,
       fromCache: PropTypes.bool,
       onCauseBadgeMouseDown: PropTypes.func.isRequired,
       onContextMenu: PropTypes.func.isRequired,
       onFocusedNodeChange: PropTypes.func,
       onMouseDown: PropTypes.func.isRequired,
       onSecurityIconMouseDown: PropTypes.func.isRequired,
-      onThumbnailMouseDown: PropTypes.func.isRequired,
       onWaterfallMouseDown: PropTypes.func.isRequired,
       waterfallWidth: PropTypes.number,
     };
   }
 
   componentDidMount() {
     if (this.props.isSelected) {
       this.refs.listItem.focus();
@@ -121,17 +120,16 @@ class RequestListItem extends Component 
       index,
       isSelected,
       firstRequestStartedMillis,
       fromCache,
       onContextMenu,
       onMouseDown,
       onCauseBadgeMouseDown,
       onSecurityIconMouseDown,
-      onThumbnailMouseDown,
       onWaterfallMouseDown,
     } = this.props;
 
     let classList = ["request-list-item", index % 2 ? "odd" : "even"];
     isSelected && classList.push("selected");
     fromCache && classList.push("fromCache");
 
     return (
@@ -140,17 +138,17 @@ class RequestListItem extends Component 
         className: classList.join(" "),
         "data-id": item.id,
         tabIndex: 0,
         onContextMenu,
         onMouseDown,
       },
         columns.get("status") && RequestListColumnStatus({ item }),
         columns.get("method") && RequestListColumnMethod({ item }),
-        columns.get("file") && RequestListColumnFile({ item, onThumbnailMouseDown }),
+        columns.get("file") && RequestListColumnFile({ item }),
         columns.get("protocol") && RequestListColumnProtocol({ item }),
         columns.get("scheme") && RequestListColumnScheme({ item }),
         columns.get("domain") && RequestListColumnDomain({ item,
                                                            onSecurityIconMouseDown }),
         columns.get("remoteip") && RequestListColumnRemoteIP({ item }),
         columns.get("cause") && RequestListColumnCause({ item, onCauseBadgeMouseDown }),
         columns.get("type") && RequestListColumnType({ item }),
         columns.get("cookies") && RequestListColumnCookies({ item }),
--- a/devtools/client/netmonitor/src/connector/chrome/response.js
+++ b/devtools/client/netmonitor/src/connector/chrome/response.js
@@ -1,16 +1,14 @@
 /* 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 { formDataURI } = require("../../utils/request-utils");
-
 function ResponseInfo(id, response, content) {
   let {
     mimeType
   } = response;
   const {body, base64Encoded} = content;
   return {
     from: id,
     content: {
@@ -18,29 +16,26 @@ function ResponseInfo(id, response, cont
       text: !body ? "" : body,
       size: !body ? 0 : body.length,
       encoding: base64Encoded ? "base64" : undefined
     }
   };
 }
 
 function ResponseContent(id, response, content) {
-  const {body, base64Encoded} = content;
+  const {body} = content;
   let {mimeType, encodedDataLength} = response;
   let responseContent = ResponseInfo(id, response, content);
   let payload = Object.assign(
     {
       responseContent,
       contentSize: !body ? 0 : body.length,
       transferredSize: encodedDataLength, // TODO: verify
       mimeType: mimeType
     }, body);
-  if (mimeType.includes("image/")) {
-    payload.responseContentDataUri = formDataURI(mimeType, base64Encoded, response);
-  }
   return payload;
 }
 
 /**
  * Not support on current version.
  * unstable method: Security
  * cause: https://chromedevtools.github.io/devtools-protocol/tot/Security/
  */
--- a/devtools/client/netmonitor/src/connector/firefox-data-provider.js
+++ b/devtools/client/netmonitor/src/connector/firefox-data-provider.js
@@ -4,17 +4,16 @@
 /* eslint-disable block-scoped-var */
 
 "use strict";
 
 const { EVENTS } = require("../constants");
 const { CurlUtils } = require("devtools/client/shared/curl");
 const {
   fetchHeaders,
-  formDataURI,
 } = require("../utils/request-utils");
 
 /**
  * This object is responsible for fetching additional HTTP
  * data from the backend over RDP protocol.
  *
  * The object also keeps track of RDP requests in-progress,
  * so it's possible to determine whether all has been fetched
@@ -123,23 +122,18 @@ class FirefoxDataProvider {
     this.pushRequestToQueue(id, payload);
 
     return payload;
   }
 
   async fetchResponseContent(mimeType, responseContent) {
     let payload = {};
     if (mimeType && responseContent && responseContent.content) {
-      let { encoding, text } = responseContent.content;
+      let { text } = responseContent.content;
       let response = await this.getLongString(text);
-
-      if (mimeType.includes("image/")) {
-        payload.responseContentDataUri = formDataURI(mimeType, encoding, response);
-      }
-
       responseContent.content.text = response;
       payload.responseContent = responseContent;
     }
     return payload;
   }
 
   async fetchRequestHeaders(requestHeaders) {
     let payload = {};
--- a/devtools/client/netmonitor/src/reducers/requests.js
+++ b/devtools/client/netmonitor/src/reducers/requests.js
@@ -54,17 +54,16 @@ const Request = I.Record({
   requestHeaders: undefined,
   requestHeadersFromUploadStream: undefined,
   requestCookies: undefined,
   requestPostData: undefined,
   responseHeaders: undefined,
   responseCookies: undefined,
   responseContent: undefined,
   responseContentAvailable: false,
-  responseContentDataUri: undefined,
   formDataSections: undefined,
 });
 
 const Requests = I.Record({
   // The collection of requests (keyed by id)
   requests: I.Map(),
   // Selection state
   selectedId: null,
--- a/devtools/client/netmonitor/src/request-list-tooltip.js
+++ b/devtools/client/netmonitor/src/request-list-tooltip.js
@@ -8,27 +8,28 @@ const {
   setImageTooltip,
   getImageDimensions,
 } = require("devtools/client/shared/widgets/tooltip/ImageTooltipHelper");
 const { formDataURI } = require("./utils/request-utils");
 
 const REQUESTS_TOOLTIP_IMAGE_MAX_DIM = 400; // px
 
 async function setTooltipImageContent(connector, tooltip, itemEl, requestItem) {
-  let { mimeType, text, encoding } = requestItem.responseContent.content;
+  let { mimeType } = requestItem;
 
   if (!mimeType || !mimeType.includes("image/")) {
     return false;
   }
 
-  let string = await connector.getLongString(text);
-  let src = formDataURI(mimeType, encoding, string);
+  let responseContent = await connector.requestData(requestItem.id, "responseContent");
+  let { encoding, text } = responseContent.content;
+  let src = formDataURI(mimeType, encoding, text);
   let maxDim = REQUESTS_TOOLTIP_IMAGE_MAX_DIM;
   let { naturalWidth, naturalHeight } = await getImageDimensions(tooltip.doc, src);
   let options = { maxDim, naturalWidth, naturalHeight };
   setImageTooltip(tooltip, tooltip.doc, src, options);
 
-  return itemEl.querySelector(".requests-list-icon");
+  return itemEl.querySelector(".requests-list-file");
 }
 
 module.exports = {
   setTooltipImageContent,
 };
--- a/devtools/client/netmonitor/test/browser.ini
+++ b/devtools/client/netmonitor/test/browser.ini
@@ -117,17 +117,16 @@ skip-if = (os == 'linux' && debug && bit
 [browser_net_filter-02.js]
 [browser_net_filter-03.js]
 [browser_net_filter-04.js]
 [browser_net_filter-autocomplete.js]
 [browser_net_filter-flags.js]
 [browser_net_footer-summary.js]
 [browser_net_headers-alignment.js]
 [browser_net_headers_sorted.js]
-[browser_net_icon-preview.js]
 [browser_net_image-tooltip.js]
 [browser_net_json-b64.js]
 [browser_net_json-null.js]
 [browser_net_json-long.js]
 [browser_net_json-malformed.js]
 [browser_net_json_custom_mime.js]
 [browser_net_json_text_mime.js]
 [browser_net_jsonp.js]
@@ -171,14 +170,13 @@ skip-if = true # Bug 1258809
 [browser_net_simple-request.js]
 [browser_net_sort-01.js]
 [browser_net_sort-02.js]
 [browser_net_statistics-01.js]
 [browser_net_statistics-02.js]
 [browser_net_status-codes.js]
 [browser_net_streaming-response.js]
 [browser_net_throttle.js]
-[browser_net_thumbnail-click.js]
 [browser_net_timeline_ticks.js]
 skip-if = true # TODO: fix the test
 [browser_net_timing-division.js]
 [browser_net_truncate.js]
 [browser_net_waterfall-click.js]
deleted file mode 100644
--- a/devtools/client/netmonitor/test/browser_net_icon-preview.js
+++ /dev/null
@@ -1,68 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-/**
- * Tests if image responses show a thumbnail in the requests menu.
- */
-
-add_task(function* () {
-  let { tab, monitor } = yield initNetMonitor(CONTENT_TYPE_WITHOUT_CACHE_URL);
-  const SELECTOR = ".requests-list-icon[src]";
-  info("Starting test... ");
-
-  let { document, store, windowRequire, connector } = monitor.panelWin;
-  let Actions = windowRequire("devtools/client/netmonitor/src/actions/index");
-  let { triggerActivity } = connector;
-  let { ACTIVITY_TYPE } = windowRequire("devtools/client/netmonitor/src/constants");
-
-  store.dispatch(Actions.batchEnable(false));
-
-  let wait = waitForNetworkEvents(monitor, CONTENT_TYPE_WITHOUT_CACHE_REQUESTS);
-  yield performRequests();
-  yield wait;
-  yield waitUntil(() => !!document.querySelector(SELECTOR));
-
-  info("Checking the image thumbnail when all items are shown.");
-  checkImageThumbnail();
-
-  store.dispatch(Actions.sortBy("contentSize"));
-  info("Checking the image thumbnail when all items are sorted.");
-  checkImageThumbnail();
-
-  store.dispatch(Actions.toggleRequestFilterType("images"));
-  info("Checking the image thumbnail when only images are shown.");
-  checkImageThumbnail();
-
-  info("Reloading the debuggee and performing all requests again...");
-  wait = waitForNetworkEvents(monitor, CONTENT_TYPE_WITHOUT_CACHE_REQUESTS);
-  yield reloadAndPerformRequests();
-  yield wait;
-  yield waitUntil(() => !!document.querySelector(SELECTOR));
-
-  info("Checking the image thumbnail after a reload.");
-  checkImageThumbnail();
-
-  yield teardown(monitor);
-
-  function performRequests() {
-    return ContentTask.spawn(tab.linkedBrowser, {}, function* () {
-      content.wrappedJSObject.performRequests();
-    });
-  }
-
-  function* reloadAndPerformRequests() {
-    yield triggerActivity(ACTIVITY_TYPE.RELOAD.WITH_CACHE_ENABLED);
-    yield performRequests();
-  }
-
-  function checkImageThumbnail() {
-    is(document.querySelectorAll(SELECTOR).length, 1,
-      "There should be only one image request with a thumbnail displayed.");
-    is(document.querySelector(SELECTOR).src, TEST_IMAGE_DATA_URI,
-      "The image requests-list-icon thumbnail is displayed correctly.");
-    is(document.querySelector(SELECTOR).hidden, false,
-      "The image requests-list-icon thumbnail should not be hidden.");
-  }
-});
--- a/devtools/client/netmonitor/test/browser_net_image-tooltip.js
+++ b/devtools/client/netmonitor/test/browser_net_image-tooltip.js
@@ -6,48 +6,45 @@
 const IMAGE_TOOLTIP_URL = EXAMPLE_URL + "html_image-tooltip-test-page.html";
 const IMAGE_TOOLTIP_REQUESTS = 1;
 
 /**
  * Tests if image responses show a popup in the requests menu when hovered.
  */
 add_task(function* test() {
   let { tab, monitor } = yield initNetMonitor(IMAGE_TOOLTIP_URL);
-  const SELECTOR = ".requests-list-icon[src]";
   info("Starting test... ");
 
   let { document, store, windowRequire, connector } = monitor.panelWin;
   let Actions = windowRequire("devtools/client/netmonitor/src/actions/index");
   let { triggerActivity } = connector;
   let { ACTIVITY_TYPE } = windowRequire("devtools/client/netmonitor/src/constants");
   let toolboxDoc = monitor.panelWin.parent.document;
 
   store.dispatch(Actions.batchEnable(false));
 
   let onEvents = waitForNetworkEvents(monitor, IMAGE_TOOLTIP_REQUESTS);
   yield performRequests();
   yield onEvents;
-  yield waitUntil(() => !!document.querySelector(SELECTOR));
 
   info("Checking the image thumbnail after a few requests were made...");
   yield showTooltipAndVerify(document.querySelectorAll(".request-list-item")[0]);
 
   // Hide tooltip before next test, to avoid the situation that tooltip covers
   // the icon for the request of the next test.
   info("Checking the image thumbnail gets hidden...");
   yield hideTooltipAndVerify(document.querySelectorAll(".request-list-item")[0]);
 
   // +1 extra document reload
   onEvents = waitForNetworkEvents(monitor, IMAGE_TOOLTIP_REQUESTS + 1);
 
   info("Reloading the debuggee and performing all requests again...");
   yield triggerActivity(ACTIVITY_TYPE.RELOAD.WITH_CACHE_ENABLED);
   yield performRequests();
   yield onEvents;
-  yield waitUntil(() => !!document.querySelector(SELECTOR));
 
   info("Checking the image thumbnail after a reload.");
   yield showTooltipAndVerify(document.querySelectorAll(".request-list-item")[1]);
 
   info("Checking if the image thumbnail is hidden when mouse leaves the menu widget");
   let requestsListContents = document.querySelector(".requests-list-contents");
   EventUtils.synthesizeMouse(requestsListContents, 0, 0, { type: "mousemove" },
                              monitor.panelWin);
@@ -61,17 +58,17 @@ add_task(function* test() {
     });
   }
 
   /**
    * Show a tooltip on the {target} and verify that it was displayed
    * with the expected content.
    */
   function* showTooltipAndVerify(target) {
-    let anchor = target.querySelector(".requests-list-icon");
+    let anchor = target.querySelector(".requests-list-file");
     yield showTooltipOn(anchor);
 
     info("Tooltip was successfully opened for the image request.");
     is(toolboxDoc.querySelector(".tooltip-panel img").src, TEST_IMAGE_DATA_URI,
       "The tooltip's image content is displayed correctly.");
   }
 
   /**
deleted file mode 100644
--- a/devtools/client/netmonitor/test/browser_net_thumbnail-click.js
+++ /dev/null
@@ -1,40 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-/**
- * Test that clicking on the file thumbnail opens the response details tab.
- */
-
-add_task(function* () {
-  let { tab, monitor } = yield initNetMonitor(CONTENT_TYPE_WITHOUT_CACHE_URL);
-  let { document } = monitor.panelWin;
-
-  yield performRequestsAndWait();
-
-  let wait = waitForDOM(document, "#response-panel");
-
-  let request = document.querySelectorAll(".request-list-item")[5];
-  let icon = request.querySelector(".requests-list-icon");
-
-  info("Clicking thumbnail of the sixth request.");
-  EventUtils.synthesizeMouseAtCenter(icon, {}, monitor.panelWin);
-
-  yield wait;
-
-  ok(document.querySelector("#response-tab[aria-selected=true]"),
-     "Response tab is selected.");
-  ok(document.querySelector(".response-image-box"),
-     "Response image preview is shown.");
-
-  yield teardown(monitor);
-
-  function* performRequestsAndWait() {
-    let onAllEvents = waitForNetworkEvents(monitor, CONTENT_TYPE_WITHOUT_CACHE_REQUESTS);
-    yield ContentTask.spawn(tab.linkedBrowser, {}, function* () {
-      content.wrappedJSObject.performRequests();
-    });
-    yield onAllEvents;
-  }
-});