Bug 1319950 - Only clear properties on resize if device is active. r=gl

If a device is active, remove it on resize.  If not, leave device properties
alone, so that things like touch simulation stay enabled when resizing without
a device.

MozReview-Commit-ID: Hvo6AdTJRBJ

--HG--
extra : rebase_source : d8c49b55c01ca625b7e85c52c4be63175ba98fd0

devtools/client/responsive.html/actions/index.js

@@ -53,6 +53,9 @@ createEnum([
   // Indicates that the device list has been loaded successfully
   "LOAD_DEVICE_LIST_END",
 
+  // Remove the viewport's device assocation.
+  "REMOVE_DEVICE",
+
   // Resize the viewport.
   "RESIZE_VIEWPORT",
 

devtools/client/responsive.html/actions/viewports.js

@@ -8,6 +8,7 @@ const {
   ADD_VIEWPORT,
   CHANGE_DEVICE,
   CHANGE_PIXEL_RATIO,
+  REMOVE_DEVICE,
   RESIZE_VIEWPORT,
   ROTATE_VIEWPORT
 } = require("./index");
@@ -45,6 +46,16 @@ module.exports = {
     };
   },
 
+  /**
+   * Remove the viewport's device assocation.
+   */
+  removeDevice(id) {
+    return {
+      type: REMOVE_DEVICE,
+      id,
+    };
+  },
+
   /**
    * Resize the viewport.
    */

devtools/client/responsive.html/app.js

@@ -21,8 +21,9 @@ const { changeTouchSimulation } = require("./actions/touch-simulation");
 const {
   changeDevice,
   changePixelRatio,
+  removeDevice,
   resizeViewport,
-  rotateViewport
+  rotateViewport,
 } = require("./actions/viewports");
 const DeviceModal = createFactory(require("./components/device-modal"));
 const GlobalToolbar = createFactory(require("./components/global-toolbar"));
@@ -98,6 +99,16 @@ let App = createClass({
     window.postMessage({ type: "exit" }, "*");
   },
 
+  onRemoveDevice(id) {
+    // TODO: Bug 1332754: Move messaging and logic into the action creator.
+    window.postMessage({
+      type: "remove-device",
+    }, "*");
+    this.props.dispatch(removeDevice(id));
+    this.props.dispatch(changeTouchSimulation(false));
+    this.props.dispatch(changePixelRatio(id, 0));
+  },
+
   onResizeViewport(id, width, height) {
     this.props.dispatch(resizeViewport(id, width, height));
   },
@@ -138,6 +149,7 @@ let App = createClass({
       onContentResize,
       onDeviceListUpdate,
       onExit,
+      onRemoveDevice,
       onResizeViewport,
       onRotateViewport,
       onScreenshot,
@@ -179,6 +191,7 @@ let App = createClass({
         onBrowserMounted,
         onChangeDevice,
         onContentResize,
+        onRemoveDevice,
         onRotateViewport,
         onResizeViewport,
         onUpdateDeviceModalOpen,

devtools/client/responsive.html/components/resizable-viewport.js

@@ -30,6 +30,7 @@ module.exports = createClass({
     onBrowserMounted: PropTypes.func.isRequired,
     onChangeDevice: PropTypes.func.isRequired,
     onContentResize: PropTypes.func.isRequired,
+    onRemoveDevice: PropTypes.func.isRequired,
     onResizeViewport: PropTypes.func.isRequired,
     onRotateViewport: PropTypes.func.isRequired,
     onUpdateDeviceModalOpen: PropTypes.func.isRequired,
@@ -107,9 +108,14 @@ module.exports = createClass({
     // Update the viewport store with the new width and height.
     this.props.onResizeViewport(width, height);
     // Change the device selector back to an unselected device
-    // TODO: Bug 1313140: We should avoid calling this for every resize event, since it
-    // triggers RDP calls each time.
-    this.props.onChangeDevice({ name: "" });
+    // TODO: Bug 1332754: Logic like this probably belongs in the action creator.
+    if (this.props.viewport.device) {
+      // In bug 1329843 and others, we may eventually stop this approach of removing the
+      // the properties of the device on resize.  However, at the moment, there is no
+      // way to edit dPR when a device is selected, and there is no UI at all for editing
+      // UA, so it's important to keep doing this for now.
+      this.props.onRemoveDevice();
+    }
 
     this.setState({
       lastClientX,

devtools/client/responsive.html/components/viewport-dimension.js

@@ -15,7 +15,7 @@ module.exports = createClass({
 
   propTypes: {
     viewport: PropTypes.shape(Types.viewport).isRequired,
-    onChangeDevice: PropTypes.func.isRequired,
+    onRemoveDevice: PropTypes.func.isRequired,
     onResizeViewport: PropTypes.func.isRequired,
   },
 
@@ -114,7 +114,10 @@ module.exports = createClass({
     }
 
     // Change the device selector back to an unselected device
-    this.props.onChangeDevice({ name: "" });
+    // TODO: Bug 1332754: Logic like this probably belongs in the action creator.
+    if (this.props.viewport.device) {
+      this.props.onRemoveDevice();
+    }
     this.props.onResizeViewport(parseInt(this.state.width, 10),
                                 parseInt(this.state.height, 10));
   },

devtools/client/responsive.html/components/viewport.js

@@ -24,6 +24,7 @@ module.exports = createClass({
     onBrowserMounted: PropTypes.func.isRequired,
     onChangeDevice: PropTypes.func.isRequired,
     onContentResize: PropTypes.func.isRequired,
+    onRemoveDevice: PropTypes.func.isRequired,
     onResizeViewport: PropTypes.func.isRequired,
     onRotateViewport: PropTypes.func.isRequired,
     onUpdateDeviceModalOpen: PropTypes.func.isRequired,
@@ -38,6 +39,15 @@ module.exports = createClass({
     onChangeDevice(viewport.id, device);
   },
 
+  onRemoveDevice() {
+    let {
+      viewport,
+      onRemoveDevice,
+    } = this.props;
+
+    onRemoveDevice(viewport.id);
+  },
+
   onResizeViewport(width, height) {
     let {
       viewport,
@@ -70,6 +80,7 @@ module.exports = createClass({
 
     let {
       onChangeDevice,
+      onRemoveDevice,
       onRotateViewport,
       onResizeViewport,
     } = this;
@@ -80,7 +91,7 @@ module.exports = createClass({
       },
       ViewportDimension({
         viewport,
-        onChangeDevice,
+        onRemoveDevice,
         onResizeViewport,
       }),
       ResizableViewport({
@@ -92,6 +103,7 @@ module.exports = createClass({
         onBrowserMounted,
         onChangeDevice,
         onContentResize,
+        onRemoveDevice,
         onResizeViewport,
         onRotateViewport,
         onUpdateDeviceModalOpen,

devtools/client/responsive.html/components/viewports.js

@@ -22,6 +22,7 @@ module.exports = createClass({
     onBrowserMounted: PropTypes.func.isRequired,
     onChangeDevice: PropTypes.func.isRequired,
     onContentResize: PropTypes.func.isRequired,
+    onRemoveDevice: PropTypes.func.isRequired,
     onResizeViewport: PropTypes.func.isRequired,
     onRotateViewport: PropTypes.func.isRequired,
     onUpdateDeviceModalOpen: PropTypes.func.isRequired,
@@ -36,6 +37,7 @@ module.exports = createClass({
       onBrowserMounted,
       onChangeDevice,
       onContentResize,
+      onRemoveDevice,
       onResizeViewport,
       onRotateViewport,
       onUpdateDeviceModalOpen,
@@ -56,6 +58,7 @@ module.exports = createClass({
           onBrowserMounted,
           onChangeDevice,
           onContentResize,
+          onRemoveDevice,
           onResizeViewport,
           onRotateViewport,
           onUpdateDeviceModalOpen,

devtools/client/responsive.html/manager.js

@@ -467,6 +467,9 @@ ResponsiveUI.prototype = {
       case "exit":
         this.onExit();
         break;
+      case "remove-device":
+        this.onRemoveDevice(event);
+        break;
     }
   },
 
@@ -509,6 +512,14 @@ ResponsiveUI.prototype = {
     ResponsiveUIManager.closeIfNeeded(browserWindow, tab);
   },
 
+  onRemoveDevice: Task.async(function* (event) {
+    yield this.updateUserAgent();
+    yield this.updateDPPX();
+    yield this.updateTouchSimulation();
+    // Used by tests
+    this.emit("device-removed");
+  }),
+
   updateDPPX: Task.async(function* (dppx) {
     if (!dppx) {
       yield this.emulationFront.clearDPPXOverride();

devtools/client/responsive.html/reducers/viewports.js

@@ -8,6 +8,7 @@ const {
   ADD_VIEWPORT,
   CHANGE_DEVICE,
   CHANGE_PIXEL_RATIO,
+  REMOVE_DEVICE,
   RESIZE_VIEWPORT,
   ROTATE_VIEWPORT,
 } = require("../actions/index");
@@ -61,6 +62,18 @@ let reducers = {
     });
   },
 
+  [REMOVE_DEVICE](viewports, { id }) {
+    return viewports.map(viewport => {
+      if (viewport.id !== id) {
+        return viewport;
+      }
+
+      return Object.assign({}, viewport, {
+        device: "",
+      });
+    });
+  },
+
   [RESIZE_VIEWPORT](viewports, { id, width, height }) {
     return viewports.map(viewport => {
       if (viewport.id !== id) {

devtools/client/responsive.html/test/browser/browser.ini

@@ -38,6 +38,7 @@ support-files =
 [browser_toolbox_computed_view.js]
 [browser_toolbox_rule_view.js]
 [browser_toolbox_swap_browsers.js]
+[browser_touch_device.js]
 [browser_touch_simulation.js]
 [browser_viewport_basics.js]
 [browser_window_close.js]

devtools/client/responsive.html/test/browser/browser_device_change.js

@@ -40,7 +40,7 @@ addRDMTask(TEST_URL, function* ({ ui, manager }) {
   yield testUserAgent(ui, DEFAULT_UA);
   yield testDevicePixelRatio(ui, DEFAULT_DPPX);
   yield testTouchEventsOverride(ui, false);
-  testViewportSelectLabel(ui, "no device selected");
+  testViewportDeviceSelectLabel(ui, "no device selected");
 
   // Test device with custom properties
   yield selectDevice(ui, "Fake Phone RDM Test");
@@ -50,14 +50,14 @@ addRDMTask(TEST_URL, function* ({ ui, manager }) {
   yield testTouchEventsOverride(ui, true);
 
   // Test resetting device when resizing viewport
-  let deviceChanged = once(ui, "device-changed");
+  let deviceRemoved = once(ui, "device-removed");
   yield testViewportResize(ui, ".viewport-vertical-resize-handle",
     [-10, -10], [testDevice.width, testDevice.height - 10], [0, -10], ui);
-  yield deviceChanged;
+  yield deviceRemoved;
   yield testUserAgent(ui, DEFAULT_UA);
   yield testDevicePixelRatio(ui, DEFAULT_DPPX);
   yield testTouchEventsOverride(ui, false);
-  testViewportSelectLabel(ui, "no device selected");
+  testViewportDeviceSelectLabel(ui, "no device selected");
 
   // Test device with generic properties
   yield selectDevice(ui, "Laptop (1366 x 768)");
@@ -76,12 +76,6 @@ function testViewportDimensions(ui, w, h) {
      `${h}px`, `Viewport should have height of ${h}px`);
 }
 
-function testViewportSelectLabel(ui, expected) {
-  let select = ui.toolWindow.document.querySelector(".viewport-device-selector");
-  is(select.selectedOptions[0].textContent, expected,
-     `Select label should be changed to ${expected}`);
-}
-
 function* testUserAgent(ui, expected) {
   let ua = yield ContentTask.spawn(ui.getViewportBrowser(), {}, function* () {
     return content.navigator.userAgent;
@@ -94,17 +88,6 @@ function* testDevicePixelRatio(ui, expected) {
   is(dppx, expected, `devicePixelRatio should be set to ${expected}`);
 }
 
-function* testTouchEventsOverride(ui, expected) {
-  let { document } = ui.toolWindow;
-  let touchButton = document.querySelector("#global-touch-simulation-button");
-
-  let flag = yield ui.emulationFront.getTouchEventsOverride();
-  is(flag === Ci.nsIDocShell.TOUCHEVENTS_OVERRIDE_ENABLED, expected,
-    `Touch events override should be ${expected ? "enabled" : "disabled"}`);
-  is(touchButton.classList.contains("active"), expected,
-    `Touch simulation button should be ${expected ? "" : "not"} active.`);
-}
-
 function* getViewportDevicePixelRatio(ui) {
   return yield ContentTask.spawn(ui.getViewportBrowser(), {}, function* () {
     return content.devicePixelRatio;

devtools/client/responsive.html/test/browser/browser_dpr_change.js

@@ -3,7 +3,7 @@ http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-// Tests changing viewport device
+// Tests changing viewport DPR
 const TEST_URL = "data:text/html;charset=utf-8,DPR list test";
 const DEFAULT_DPPX = window.devicePixelRatio;
 const VIEWPORT_DPPX = DEFAULT_DPPX + 2;
@@ -67,8 +67,10 @@ function* testResetWhenResizingViewport(ui) {
 
   let waitPixelRatioChange = onceDevicePixelRatioChange(ui);
 
+  let deviceRemoved = once(ui, "device-removed");
   yield testViewportResize(ui, ".viewport-vertical-resize-handle",
     [-10, -10], [testDevice.width, testDevice.height - 10], [0, -10], ui);
+  yield deviceRemoved;
 
   yield waitPixelRatioChange;
   yield testDevicePixelRatio(ui, window.devicePixelRatio);
@@ -99,14 +101,6 @@ function testViewportDPRSelect(ui, expected) {
     `DPR Select should be ${expected.disabled ? "disabled" : "enabled"}.`);
 }
 
-function testViewportDeviceSelectLabel(ui, expected) {
-  info("Test viewport's device select label");
-
-  let select = ui.toolWindow.document.querySelector(".viewport-device-selector");
-  is(select.selectedOptions[0].textContent, expected,
-     `Device Select value should be: ${expected}`);
-}
-
 function* testDevicePixelRatio(ui, expected) {
   info("Test device pixel ratio");
 

devtools/client/responsive.html/test/browser/browser_touch_device.js

@@ -0,0 +1,77 @@
+/* Any copyright is dedicated to the Public Domain.
+http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Tests changing viewport touch simulation
+const TEST_URL = "data:text/html;charset=utf-8,touch simulation test";
+const Types = require("devtools/client/responsive.html/types");
+
+const testDevice = {
+  "name": "Fake Phone RDM Test",
+  "width": 320,
+  "height": 470,
+  "pixelRatio": 5.5,
+  "userAgent": "Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0",
+  "touch": true,
+  "firefoxOS": true,
+  "os": "custom",
+  "featured": true,
+};
+
+// Add the new device to the list
+addDeviceForTest(testDevice);
+
+addRDMTask(TEST_URL, function* ({ ui, manager }) {
+  yield waitStartup(ui);
+
+  yield testDefaults(ui);
+  yield testChangingDevice(ui);
+  yield testResizingViewport(ui, true, false);
+  yield testEnableTouchSimulation(ui);
+  yield testResizingViewport(ui, false, true);
+});
+
+function* waitStartup(ui) {
+  let { store } = ui.toolWindow;
+
+  // Wait until the viewport has been added and the device list has been loaded
+  yield waitUntilState(store, state => state.viewports.length == 1
+    && state.devices.listState == Types.deviceListState.LOADED);
+}
+
+function* testDefaults(ui) {
+  info("Test Defaults");
+
+  yield testTouchEventsOverride(ui, false);
+  testViewportDeviceSelectLabel(ui, "no device selected");
+}
+
+function* testChangingDevice(ui) {
+  info("Test Changing Device");
+
+  yield selectDevice(ui, testDevice.name);
+  yield waitForViewportResizeTo(ui, testDevice.width, testDevice.height);
+  yield testTouchEventsOverride(ui, true);
+  testViewportDeviceSelectLabel(ui, testDevice.name);
+}
+
+function* testResizingViewport(ui, device, expected) {
+  info(`Test resizing the viewport, device ${device}, expected ${expected}`);
+
+  let deviceRemoved = once(ui, "device-removed");
+  yield testViewportResize(ui, ".viewport-vertical-resize-handle",
+    [-10, -10], [testDevice.width, testDevice.height - 10], [0, -10], ui);
+  if (device) {
+    yield deviceRemoved;
+  }
+  yield testTouchEventsOverride(ui, expected);
+  testViewportDeviceSelectLabel(ui, "no device selected");
+}
+
+function* testEnableTouchSimulation(ui) {
+  info("Test enabling touch simulation via button");
+
+  yield enableTouchSimulation(ui);
+  yield testTouchEventsOverride(ui, true);
+}

devtools/client/responsive.html/test/browser/browser_touch_simulation.js

@@ -171,18 +171,18 @@ function testTouchButton(ui) {
   let { document } = ui.toolWindow;
   let touchButton = document.querySelector("#global-touch-simulation-button");
 
-  ok(!touchButton.classList.contains("active"),
-    "Touch simulation is not active by default.");
-
-  touchButton.click();
-
   ok(touchButton.classList.contains("active"),
-    "Touch simulation is started on click.");
+    "Touch simulation is active at end of test.");
 
   touchButton.click();
 
   ok(!touchButton.classList.contains("active"),
     "Touch simulation is stopped on click.");
+
+  touchButton.click();
+
+  ok(touchButton.classList.contains("active"),
+    "Touch simulation is started on click.");
 }
 
 function* waitBootstrap(ui) {
@@ -226,15 +226,3 @@ function* injectEventUtils(ui) {
       "chrome://mochikit/content/tests/SimpleTest/EventUtils.js", EventUtils);
   });
 }
-
-const enableTouchSimulation = ui => new Promise(
-  Task.async(function* (resolve) {
-    let browser = ui.getViewportBrowser();
-
-    browser.addEventListener("mozbrowserloadend", function onLoad() {
-      browser.removeEventListener("mozbrowserloadend", onLoad);
-      resolve();
-    });
-
-    yield ui.updateTouchSimulation(true);
-  }));

devtools/client/responsive.html/test/browser/head.js

@@ -212,12 +212,9 @@ function dragElementBy(selector, x, y, win) {
 function* testViewportResize(ui, selector, moveBy,
                              expectedViewportSize, expectedHandleMove) {
   let win = ui.toolWindow;
-
-  let changed = once(ui, "device-changed");
   let resized = waitForViewportResizeTo(ui, ...expectedViewportSize);
   let startRect = dragElementBy(selector, ...moveBy, win);
   yield resized;
-  yield changed;
 
   let endRect = getElRect(selector, win);
   is(endRect.left - startRect.left, expectedHandleMove[0],
@@ -329,6 +326,15 @@ function waitForPageShow(browser) {
   });
 }
 
+function waitForViewportLoad(ui) {
+  return new Promise(resolve => {
+    let browser = ui.getViewportBrowser();
+    browser.addEventListener("mozbrowserloadend", () => {
+      resolve();
+    }, { once: true });
+  });
+}
+
 function load(browser, url) {
   let loaded = BrowserTestUtils.browserLoaded(browser, false, url);
   browser.loadURI(url, null, null);
@@ -363,3 +369,30 @@ function waitForClientClose(ui) {
     ui.client.addOneTimeListener("closed", resolve);
   });
 }
+
+function* testTouchEventsOverride(ui, expected) {
+  let { document } = ui.toolWindow;
+  let touchButton = document.querySelector("#global-touch-simulation-button");
+
+  let flag = yield ui.emulationFront.getTouchEventsOverride();
+  is(flag === Ci.nsIDocShell.TOUCHEVENTS_OVERRIDE_ENABLED, expected,
+    `Touch events override should be ${expected ? "enabled" : "disabled"}`);
+  is(touchButton.classList.contains("active"), expected,
+    `Touch simulation button should be ${expected ? "" : "in"}active.`);
+}
+
+function testViewportDeviceSelectLabel(ui, expected) {
+  info("Test viewport's device select label");
+
+  let select = ui.toolWindow.document.querySelector(".viewport-device-selector");
+  is(select.selectedOptions[0].textContent, expected,
+     `Device Select value should be: ${expected}`);
+}
+
+function* enableTouchSimulation(ui) {
+  let { document } = ui.toolWindow;
+  let touchButton = document.querySelector("#global-touch-simulation-button");
+  let loaded = waitForViewportLoad(ui);
+  touchButton.click();
+  yield loaded;
+}