From 6f6bc3bbbf09eb0e23e1ed44ed2a8d668a485148 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 4 Jan 2018 21:04:17 -0600 Subject: [PATCH] Bug 1381463 - Reload to clear UA when exiting RDM. r=ochameau When exiting RDM with a device applied, we clear the UA override, but we did not force a refresh of the page. This means that the customized UA was still accessible via `navigator.userAgent` and the page might have a non-default layout if it tests the UA value. To ensure there's no chance of confusion after exiting RDM, force a reload if clearing emulation values determines it is needed. MozReview-Commit-ID: Fkji12Utmis --HG-- extra : rebase_source : a7ae8851165fae7aec5875d10f51cbb703b3f870 --- devtools/client/responsive.html/manager.js | 102 +++++++++++++----- .../test/browser/browser_device_change.js | 45 +++++++- .../responsive.html/test/browser/head.js | 15 ++- 3 files changed, 122 insertions(+), 40 deletions(-) diff --git a/devtools/client/responsive.html/manager.js b/devtools/client/responsive.html/manager.js index 0327ef981484..3b17b00d4828 100644 --- a/devtools/client/responsive.html/manager.js +++ b/devtools/client/responsive.html/manager.js @@ -373,7 +373,7 @@ ResponsiveUI.prototype = { * Whether this call is actually destroying. False means destruction * was already in progress. */ - destroy: Task.async(function* (options) { + async destroy(options) { if (this.destroying) { return false; } @@ -389,7 +389,7 @@ ResponsiveUI.prototype = { // Ensure init has finished before starting destroy if (!isTabContentDestroying) { - yield this.inited; + await this.inited; } this.tab.removeEventListener("TabClose", this); @@ -399,7 +399,20 @@ ResponsiveUI.prototype = { if (!isTabContentDestroying) { // Notify the inner browser to stop the frame script - yield message.request(this.toolWindow, "stop-frame-script"); + await message.request(this.toolWindow, "stop-frame-script"); + } + + // Ensure the tab is reloaded if required when exiting RDM so that no emulated + // settings are left in a customized state. + if (!isTabContentDestroying) { + let reloadNeeded = false; + reloadNeeded |= await this.updateDPPX(); + reloadNeeded |= await this.updateNetworkThrottling(); + reloadNeeded |= await this.updateUserAgent(); + reloadNeeded |= await this.updateTouchSimulation(); + if (reloadNeeded) { + this.getViewportBrowser().reload(); + } } // Destroy local state @@ -415,7 +428,7 @@ ResponsiveUI.prototype = { // anything on shutdown client side. let clientClosed = this.client.close(); if (!isTabContentDestroying) { - yield clientClosed; + await clientClosed; } this.client = this.emulationFront = null; @@ -427,7 +440,7 @@ ResponsiveUI.prototype = { this.destroyed = true; return true; - }), + }, connectToServer: Task.async(function* () { DebuggerServer.init(); @@ -487,9 +500,13 @@ ResponsiveUI.prototype = { onChangeDevice: Task.async(function* (event) { let { userAgent, pixelRatio, touch } = event.data.device; + // Bug 1428799: Should we reload on UA change as well? yield this.updateUserAgent(userAgent); yield this.updateDPPX(pixelRatio); - yield this.updateTouchSimulation(touch); + let reloadNeeded = yield this.updateTouchSimulation(touch); + if (reloadNeeded) { + this.getViewportBrowser().reload(); + } // Used by tests this.emit("device-changed"); }), @@ -506,9 +523,12 @@ ResponsiveUI.prototype = { this.updateDPPX(pixelRatio); }, - onChangeTouchSimulation(event) { + async onChangeTouchSimulation(event) { let { enabled } = event.data; - this.updateTouchSimulation(enabled); + let reloadNeeded = await this.updateTouchSimulation(enabled); + if (reloadNeeded) { + this.getViewportBrowser().reload(); + } // Used by tests this.emit("touch-simulation-changed"); }, @@ -527,25 +547,44 @@ ResponsiveUI.prototype = { }, onRemoveDeviceAssociation: Task.async(function* (event) { + // Bug 1428799: Should we reload on UA change as well? yield this.updateUserAgent(); yield this.updateDPPX(); - yield this.updateTouchSimulation(); + let reloadNeeded = yield this.updateTouchSimulation(); + if (reloadNeeded) { + this.getViewportBrowser().reload(); + } // Used by tests this.emit("device-association-removed"); }), + /** + * Set or clear the emulated device pixel ratio. + * + * @return boolean + * Whether a reload is needed to apply the change. + * (This is always immediate, so it's always false.) + */ updateDPPX: Task.async(function* (dppx) { if (!dppx) { yield this.emulationFront.clearDPPXOverride(); - return; + return false; } yield this.emulationFront.setDPPXOverride(dppx); + return false; }), + /** + * Set or clear network throttling. + * + * @return boolean + * Whether a reload is needed to apply the change. + * (This is always immediate, so it's always false.) + */ updateNetworkThrottling: Task.async(function* (enabled, profile) { if (!enabled) { yield this.emulationFront.clearNetworkThrottling(); - return; + return false; } let data = throttlingProfiles.find(({ id }) => id == profile); let { download, upload, latency } = data; @@ -554,29 +593,36 @@ ResponsiveUI.prototype = { uploadThroughput: upload, latency, }); + return false; }), - updateUserAgent: Task.async(function* (userAgent) { + /** + * Set or clear the emulated user agent. + * + * @return boolean + * Whether a reload is needed to apply the change. + */ + updateUserAgent(userAgent) { if (!userAgent) { - yield this.emulationFront.clearUserAgentOverride(); - return; + return this.emulationFront.clearUserAgentOverride(); } - yield this.emulationFront.setUserAgentOverride(userAgent); - }), + return this.emulationFront.setUserAgentOverride(userAgent); + }, - updateTouchSimulation: Task.async(function* (enabled) { - let reloadNeeded; - if (enabled) { - reloadNeeded = yield this.emulationFront.setTouchEventsOverride( - Ci.nsIDocShell.TOUCHEVENTS_OVERRIDE_ENABLED - ); - } else { - reloadNeeded = yield this.emulationFront.clearTouchEventsOverride(); + /** + * Set or clear touch simulation. + * + * @return boolean + * Whether a reload is needed to apply the change. + */ + updateTouchSimulation(enabled) { + if (!enabled) { + return this.emulationFront.clearTouchEventsOverride(); } - if (reloadNeeded) { - this.getViewportBrowser().reload(); - } - }), + return this.emulationFront.setTouchEventsOverride( + Ci.nsIDocShell.TOUCHEVENTS_OVERRIDE_ENABLED + ); + }, /** * Helper for tests. Assumes a single viewport for now. diff --git a/devtools/client/responsive.html/test/browser/browser_device_change.js b/devtools/client/responsive.html/test/browser/browser_device_change.js index 3f62976e1246..1215f4687c0d 100644 --- a/devtools/client/responsive.html/test/browser/browser_device_change.js +++ b/devtools/client/responsive.html/test/browser/browser_device_change.js @@ -3,8 +3,8 @@ http://creativecommons.org/publicdomain/zero/1.0/ */ "use strict"; -// Tests changing viewport device -const TEST_URL = "data:text/html;charset=utf-8,Device list test"; +// Tests changing viewport device (need HTTP load for proper UA testing) +const TEST_URL = `${URL_ROOT}doc_page_state.html`; const DEFAULT_DPPX = window.devicePixelRatio; const DEFAULT_UA = Cc["@mozilla.org/network/protocol;1?name=http"] @@ -28,7 +28,7 @@ const testDevice = { // Add the new device to the list addDeviceForTest(testDevice); -addRDMTask(TEST_URL, function* ({ ui, manager }) { +addRDMTask(TEST_URL, function* ({ ui }) { let { store } = ui.toolWindow; // Wait until the viewport has been added and the device list has been loaded @@ -37,23 +37,29 @@ addRDMTask(TEST_URL, function* ({ ui, manager }) { // Test defaults testViewportDimensions(ui, 320, 480); + info("Should have default UA at the start of the test"); yield testUserAgent(ui, DEFAULT_UA); yield testDevicePixelRatio(ui, DEFAULT_DPPX); yield testTouchEventsOverride(ui, false); testViewportDeviceSelectLabel(ui, "no device selected"); // Test device with custom properties + let reloaded = waitForViewportLoad(ui); yield selectDevice(ui, "Fake Phone RDM Test"); + yield reloaded; yield waitForViewportResizeTo(ui, testDevice.width, testDevice.height); + info("Should have device UA now that device is applied"); yield testUserAgent(ui, testDevice.userAgent); yield testDevicePixelRatio(ui, testDevice.pixelRatio); yield testTouchEventsOverride(ui, true); // Test resetting device when resizing viewport let deviceRemoved = once(ui, "device-association-removed"); + reloaded = waitForViewportLoad(ui); yield testViewportResize(ui, ".viewport-vertical-resize-handle", [-10, -10], [testDevice.width, testDevice.height - 10], [0, -10], ui); - yield deviceRemoved; + yield Promise.all([ deviceRemoved, reloaded ]); + info("Should have default UA after resizing viewport"); yield testUserAgent(ui, DEFAULT_UA); yield testDevicePixelRatio(ui, DEFAULT_DPPX); yield testTouchEventsOverride(ui, false); @@ -62,11 +68,42 @@ addRDMTask(TEST_URL, function* ({ ui, manager }) { // Test device with generic properties yield selectDevice(ui, "Laptop (1366 x 768)"); yield waitForViewportResizeTo(ui, 1366, 768); + info("Should have default UA when using device without specific UA"); yield testUserAgent(ui, DEFAULT_UA); yield testDevicePixelRatio(ui, 1); yield testTouchEventsOverride(ui, false); }); +add_task(async function () { + const tab = await addTab(TEST_URL); + const { ui } = await openRDM(tab); + + let { store } = ui.toolWindow; + + // Wait until the viewport has been added and the device list has been loaded + await waitUntilState(store, state => state.viewports.length == 1 + && state.devices.listState == Types.deviceListState.LOADED); + + // Select device with custom UA + let reloaded = waitForViewportLoad(ui); + await selectDevice(ui, "Fake Phone RDM Test"); + await reloaded; + await waitForViewportResizeTo(ui, testDevice.width, testDevice.height); + info("Should have device UA now that device is applied"); + await testUserAgent(ui, testDevice.userAgent); + + // Browser will reload to clear the UA on RDM close + reloaded = waitForViewportLoad(ui); + await closeRDM(tab); + await reloaded; + + // Ensure UA is reset to default after closing RDM + info("Should have default UA after closing RDM"); + await testUserAgentFromBrowser(tab.linkedBrowser, DEFAULT_UA); + + await removeTab(tab); +}); + function testViewportDimensions(ui, w, h) { let viewport = ui.toolWindow.document.querySelector(".viewport-content"); diff --git a/devtools/client/responsive.html/test/browser/head.js b/devtools/client/responsive.html/test/browser/head.js index bc9040e91108..7170049d9ace 100644 --- a/devtools/client/responsive.html/test/browser/head.js +++ b/devtools/client/responsive.html/test/browser/head.js @@ -301,12 +301,7 @@ function waitForPageShow(browser) { } function waitForViewportLoad(ui) { - return new Promise(resolve => { - let browser = ui.getViewportBrowser(); - browser.addEventListener("mozbrowserloadend", () => { - resolve(); - }, { once: true }); - }); + return BrowserTestUtils.waitForContentEvent(ui.getViewportBrowser(), "load", true); } function load(browser, url) { @@ -371,8 +366,12 @@ function* toggleTouchSimulation(ui) { yield Promise.all([ changed, loaded ]); } -function* testUserAgent(ui, expected) { - let ua = yield ContentTask.spawn(ui.getViewportBrowser(), {}, function* () { +function testUserAgent(ui, expected) { + testUserAgentFromBrowser(ui.getViewportBrowser(), expected); +} + +async function testUserAgentFromBrowser(browser, expected) { + let ua = await ContentTask.spawn(browser, {}, function* () { return content.navigator.userAgent; }); is(ua, expected, `UA should be set to ${expected}`);