Bug 1513244 - Close autocomplete navigation on page navigation; r=Honza.

The autocomplete popup contains information about the context it was opened in. If the context changes, the content of the popup might be outdated, or plainly wrong. Closing the popup seems like a safe way to handle this. We take this as an opportunity to move the willNavigate handler to webconsole-output-wrapper, where we can dispatch a new WILL_NAVIGATE action to which reducers might react. A test is added to make sure this works as expected. Depends on D12939 Differential Revision: https://phabricator.services.mozilla.com/D14153 --HG-- extra : moz-landing-system : lando
2018-12-13 14:35:34 +00:00 · 2018-12-13 14:35:34 +00:00 · 2b41cc1612
--- a/devtools/client/webconsole/constants.js
+++ b/devtools/client/webconsole/constants.js
@ -43,6 +43,7 @@ const actionTypes = {
  REVERSE_SEARCH_NEXT: "REVERSE_SEARCH_NEXT",
  REVERSE_SEARCH_BACK: "REVERSE_SEARCH_BACK",
  PAUSED_EXCECUTION_POINT: "PAUSED_EXCECUTION_POINT",
+  WILL_NAVIGATE: "WILL_NAVIGATE",
 };

 const prefs = {
--- a/devtools/client/webconsole/reducers/autocomplete.js
+++ b/devtools/client/webconsole/reducers/autocomplete.js
@ -8,6 +8,7 @@ const {
  AUTOCOMPLETE_DATA_RECEIVE,
  AUTOCOMPLETE_PENDING_REQUEST,
  AUTOCOMPLETE_RETRIEVE_FROM_CACHE,
+  WILL_NAVIGATE,
 } = require("devtools/client/webconsole/constants");

 function getDefaultState() {
@ -23,6 +24,7 @@ function getDefaultState() {
 function autocomplete(state = getDefaultState(), action) {
  switch (action.type) {
    case AUTOCOMPLETE_CLEAR:
+    case WILL_NAVIGATE:
      return getDefaultState();
    case AUTOCOMPLETE_RETRIEVE_FROM_CACHE:
      return autoCompleteRetrieveFromCache(state, action);
--- a/devtools/client/webconsole/test/mochitest/browser.ini
+++ b/devtools/client/webconsole/test/mochitest/browser.ini
@ -205,6 +205,7 @@ skip-if = verify
 [browser_jsterm_autocomplete_return_key_no_selection.js]
 [browser_jsterm_autocomplete_return_key.js]
 [browser_jsterm_autocomplete_width.js]
+[browser_jsterm_autocomplete_will_navigate.js]
 [browser_jsterm_autocomplete-properties-with-non-alphanumeric-names.js]
 [browser_jsterm_await_concurrent.js]
 [browser_jsterm_await_error.js]
--- a/devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_will_navigate.js
+++ b/devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_will_navigate.js
@ -0,0 +1,58 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that navigating the page closes the autocomplete popup.
+
+const TEST_URI = `data:text/html;charset=utf-8,
+<head>
+  <script>
+    /* Create a prototype-less object so popup does not contain native
+     * Object prototype properties.
+     */
+    window.foo = Object.create(null, Object.getOwnPropertyDescriptors({
+      item0: "value0",
+      item1: "value1",
+    }));
+  </script>
+</head>
+<body>Test autocomplete close on content navigation</body>`;
+
+add_task(async function() {
+  // Run test with legacy JsTerm
+  await pushPref("devtools.webconsole.jsterm.codeMirror", false);
+  await performTests();
+  // And then run it with the CodeMirror-powered one.
+  await pushPref("devtools.webconsole.jsterm.codeMirror", true);
+  await performTests();
+});
+
+async function performTests() {
+  const { jsterm } = await openNewTabAndConsole(TEST_URI);
+  info("web console opened");
+
+  const { autocompletePopup: popup } = jsterm;
+
+  const onPopUpOpen = popup.once("popup-opened");
+
+  info("wait for completion: window.foo.");
+  jsterm.setInputValue("window.foo");
+  EventUtils.sendString(".");
+
+  await onPopUpOpen;
+
+  ok(popup.isOpen, "popup is open");
+  ok(popup.itemCount, "popup has items");
+
+  info("reload the page to close the popup");
+  const onPopupClose = popup.once("popup-closed");
+  await refreshTab();
+  await onPopupClose;
+
+  ok(!popup.isOpen, "popup is not open after reloading the page");
+  is(jsterm.getInputValue(), "window.foo.", "completion was cancelled");
+  ok(!getJsTermCompletionValue(jsterm), "completeNode is empty");
+}
--- a/devtools/client/webconsole/webconsole-frame.js
+++ b/devtools/client/webconsole/webconsole-frame.js
@ -21,7 +21,6 @@ loader.lazyRequireGetter(this, "AppConstants", "resource://gre/modules/AppConsta
 const ZoomKeys = require("devtools/client/shared/zoom-keys");

 const PREF_MESSAGE_TIMESTAMP = "devtools.webconsole.timestampMessages";
-const PREF_PERSISTLOG = "devtools.webconsole.persistlog";
 const PREF_SIDEBAR_ENABLED = "devtools.webconsole.sidebarToggle";

 /**
@ -57,19 +56,6 @@ WebConsoleFrame.prototype = {
    return this.proxy ? this.proxy.webConsoleClient : null;
  },

-  /**
-   * Getter for the persistent logging preference.
-   * @type boolean
-   */
-  get persistLog() {
-    // For the browser console, we receive tab navigation
-    // when the original top level window we attached to is closed,
-    // but we don't want to reset console history and just switch to
-    // the next available window.
-    return this.isBrowserConsole ||
-           Services.prefs.getBoolPref(PREF_PERSISTLOG);
-  },
-
  /**
   * Initialize the WebConsoleFrame instance.
   * @return object
@ -395,14 +381,7 @@ WebConsoleFrame.prototype = {
  },

  handleTabWillNavigate: function(packet) {
-    if (this.persistLog) {
-      // Add a _type to hit convertCachedPacket.
-      packet._type = true;
-      this.consoleOutput.dispatchMessageAdd(packet);
-    } else {
-      this.clearOutput(false);
-    }
-
+    this.consoleOutput.dispatchTabWillNavigate(packet);
    if (packet.url) {
      this.onLocationChange(packet.url, packet.title);
    }
--- a/devtools/client/webconsole/webconsole-output-wrapper.js
+++ b/devtools/client/webconsole/webconsole-output-wrapper.js
@ -22,6 +22,7 @@ const EventEmitter = require("devtools/shared/event-emitter");
 const App = createFactory(require("devtools/client/webconsole/components/App"));
 const ObjectClient = require("devtools/shared/client/object-client");
 const LongStringClient = require("devtools/shared/client/long-string-client");
+loader.lazyRequireGetter(this, "Constants", "devtools/client/webconsole/constants");

 let store = null;

@ -366,6 +367,7 @@ WebConsoleOutputWrapper.prototype = {
    this.queuedMessageUpdates = [];
    this.queuedRequestUpdates = [];
    store.dispatch(actions.messagesClear());
+    this.hud.emit("messages-cleared");
  },

  dispatchPrivateMessagesClear: function() {
@ -464,6 +466,26 @@ WebConsoleOutputWrapper.prototype = {
      this.toolbox && this.toolbox.currentToolId !== "webconsole"));
  },

+  dispatchTabWillNavigate: function(packet) {
+    const { ui } = store.getState();
+
+    // For the browser console, we receive tab navigation
+    // when the original top level window we attached to is closed,
+    // but we don't want to reset console history and just switch to
+    // the next available window.
+    if (ui.persistLogs || this.hud.isBrowserConsole) {
+      // Add a _type to hit convertCachedPacket.
+      packet._type = true;
+      this.dispatchMessageAdd(packet);
+    } else {
+      this.hud.webConsoleClient.clearNetworkRequests();
+      this.dispatchMessagesClear();
+      store.dispatch({
+        type: Constants.WILL_NAVIGATE,
+      });
+    }
+  },
+
  batchedMessageUpdates: function(info) {
    this.queuedMessageUpdates.push(info);
    this.setTimeoutIfNeeded();