Bug 1525618 - Add an option in the settings panel to toggle warning groups. r=Honza.

We also listen for the warningGroup preference change to
toggle warningGroups in the console output.
If warningGroups were disabled, we need to loop through
the state messages to create warningGroups when needed.
A test is added to ensure this works as expected.

Differential Revision: https://phabricator.services.mozilla.com/D32716

--HG--
extra : moz-landing-system : lando

devtools/client/framework/toolbox-options.xhtml

@@ -74,6 +74,12 @@
                  data-pref="devtools.webconsole.timestampMessages"/>
           <span>&options.timestampMessages.label;</span>
         </label>
+        <label title="&options.warningGroups.tooltip;">
+          <input type="checkbox"
+                 id="webconsole-warning-groups"
+                 data-pref="devtools.webconsole.groupWarningMessages"/>
+          <span>&options.warningGroups.label;</span>
+        </label>
       </fieldset>
 
       <fieldset id="debugger-options" class="options-groupbox" hidden="true">

devtools/client/locales/en-US/toolbox.dtd

@@ -136,6 +136,11 @@
 <!ENTITY options.timestampMessages.label      "Enable timestamps">
 <!ENTITY options.timestampMessages.tooltip    "If you enable this option commands and output in the Web Console will display a timestamp">
 
+<!-- LOCALIZATION NOTE (options.warningGroups.label): This is the
+   - label for the checkbox that toggles the warningGroups feature in the Web Console -->
+<!ENTITY options.warningGroups.label      "Group similar message">
+<!ENTITY options.warningGroups.tooltip    "When enabled, similar messages are placed into groups">
+
 <!-- LOCALIZATION NOTE (options.debugger.label): This is the label for the
   -  heading of the group of Debugger preferences in the options panel. -->
 <!ENTITY options.debugger.label            "Debugger">

devtools/client/webconsole/actions/ui.js

@@ -20,6 +20,7 @@ const {
   SIDEBAR_CLOSE,
   SPLIT_CONSOLE_CLOSE_BUTTON_TOGGLE,
   TIMESTAMPS_TOGGLE,
+  WARNING_GROUPS_TOGGLE,
 } = require("devtools/client/webconsole/constants");
 
 function persistToggle() {
@@ -49,6 +50,13 @@ function timestampsToggle(visible) {
   };
 }
 
+function warningGroupsToggle(value) {
+  return {
+    type: WARNING_GROUPS_TOGGLE,
+    value,
+  };
+}
+
 function selectNetworkMessageTab(id) {
   return {
     type: SELECT_NETWORK_MESSAGE_TAB,
@@ -121,4 +129,5 @@ module.exports = {
   sidebarClose,
   splitConsoleCloseButtonToggle,
   timestampsToggle,
+  warningGroupsToggle,
 };

devtools/client/webconsole/components/MessageContainer.js

@@ -43,6 +43,7 @@ class MessageContainer extends Component {
       getMessage: PropTypes.func.isRequired,
       isPaused: PropTypes.bool.isRequired,
       pausedExecutionPoint: PropTypes.any,
+      inWarningGroup: PropTypes.bool,
     };
   }
 
@@ -52,29 +53,21 @@ class MessageContainer extends Component {
     };
   }
 
-  shouldComponentUpdate(nextProps, nextState) {
-    const repeatChanged = this.props.repeat !== nextProps.repeat;
-    const openChanged = this.props.open !== nextProps.open;
-    const payloadChanged = this.props.payload !== nextProps.payload;
-    const tableDataChanged = this.props.tableData !== nextProps.tableData;
-    const timestampVisibleChanged =
-      this.props.timestampsVisible !== nextProps.timestampsVisible;
-    const networkMessageUpdateChanged =
-      this.props.networkMessageUpdate !== nextProps.networkMessageUpdate;
-    const pausedChanged = this.props.isPaused !== nextProps.isPaused;
-    const executionPointChanged =
-      this.props.pausedExecutionPoint !== nextProps.pausedExecutionPoint;
-    const badgeChanged = this.props.badge !== nextProps.badge;
+  shouldComponentUpdate(nextProps) {
+    const triggeringUpdateProps = [
+      "repeat",
+      "open",
+      "payload",
+      "tableData",
+      "timestampsVisible",
+      "networkMessageUpdate",
+      "isPaused",
+      "pausedExecutionPoint",
+      "badge",
+      "inWarningGroup",
+    ];
 
-    return repeatChanged
-      || badgeChanged
-      || openChanged
-      || payloadChanged
-      || tableDataChanged
-      || timestampVisibleChanged
-      || networkMessageUpdateChanged
-      || pausedChanged
-      || executionPointChanged;
+    return triggeringUpdateProps.some(prop => this.props[prop] !== nextProps[prop]);
   }
 
   render() {

devtools/client/webconsole/constants.js

@@ -45,6 +45,7 @@ const actionTypes = {
   REVERSE_SEARCH_NEXT: "REVERSE_SEARCH_NEXT",
   REVERSE_SEARCH_BACK: "REVERSE_SEARCH_BACK",
   PAUSED_EXCECUTION_POINT: "PAUSED_EXCECUTION_POINT",
+  WARNING_GROUPS_TOGGLE: "WARNING_GROUPS_TOGGLE",
   WILL_NAVIGATE: "WILL_NAVIGATE",
 };
 

devtools/client/webconsole/reducers/messages.js

@@ -174,7 +174,6 @@ function addMessage(newMessage, state, filtersState, prefsState, uiState) {
       const groupMessage = createWarningGroupMessage(
         warningGroupMessageId, warningGroupType, newMessage);
       state = addMessage(groupMessage, state, filtersState, prefsState, uiState);
-      state.warningGroupsById.set(warningGroupMessageId, []);
     }
 
     // We add the new message to the appropriate warningGroup.
@@ -215,6 +214,11 @@ function addMessage(newMessage, state, filtersState, prefsState, uiState) {
     }
   }
 
+  // If we're creating a warningGroup, we init the array for its children.
+  if (isWarningGroup(newMessage)) {
+    state.warningGroupsById.set(newMessage.id, []);
+  }
+
   const addedMessage = Object.freeze(newMessage);
   state.messagesById.set(newMessage.id, addedMessage);
 
@@ -508,44 +512,117 @@ function messages(state = MessageState(), action, filtersState, prefsState, uiSt
         removedActors: [],
       };
 
+    case constants.WARNING_GROUPS_TOGGLE:
+      // There's no warningGroups, and the pref was set to false,
+      // we don't need to do anything.
+      if (!prefsState.groupWarnings && state.warningGroupsById.size === 0) {
+        return state;
+      }
+
+      let needSort = false;
+      const messageEntries = state.messagesById.entries();
+      for (const [msgId, message] of messageEntries) {
+        const warningGroupType = getWarningGroupType(message);
+        if (warningGroupType) {
+          const warningGroupMessageId = getParentWarningGroupMessageId(message);
+
+          // If there's no warning group for the type/innerWindowID yet.
+          if (!state.messagesById.has(warningGroupMessageId)) {
+            // We create it and add it to the store.
+            const groupMessage = createWarningGroupMessage(
+              warningGroupMessageId, warningGroupType, message);
+            state = addMessage(groupMessage, state, filtersState, prefsState, uiState);
+          }
+
+          // We add the new message to the appropriate warningGroup.
+          const warningGroup = state.warningGroupsById.get(warningGroupMessageId);
+          if (warningGroup && !warningGroup.includes(msgId)) {
+            warningGroup.push(msgId);
+          }
+
+          needSort = true;
+        }
+      }
+
+      // If we don't have any warning messages that could be in a group, we don't do
+      // anything.
+      if (!needSort) {
+        return state;
+      }
+
+      return setVisibleMessages({
+        messagesState: state,
+        filtersState,
+        prefsState,
+        uiState,
+        // If the user disabled warning groups, we want the messages to be sorted by their
+        // timestamps.
+        forceTimestampSort: !prefsState.groupWarnings,
+      });
+
     case constants.FILTER_TOGGLE:
     case constants.FILTER_TEXT_SET:
     case constants.FILTERS_CLEAR:
     case constants.DEFAULT_FILTERS_RESET:
     case constants.SHOW_CONTENT_MESSAGES_TOGGLE:
-      const messagesToShow = [];
-      const filtered = getDefaultFiltersCounter();
-
-      messagesById.forEach((message, msgId) => {
-        const { visible, cause } = getMessageVisibility(message, {
-          messagesState: state,
-          filtersState,
-          prefsState,
-          uiState,
-        });
-
-        if (visible) {
-          messagesToShow.push(msgId);
-        } else if (DEFAULT_FILTERS.includes(cause)) {
-          filtered.global = filtered.global + 1;
-          filtered[cause] = filtered[cause] + 1;
-        }
+      return setVisibleMessages({
+        messagesState: state,
+        filtersState,
+        prefsState,
+        uiState,
       });
-
-      const filteredState = {
-        ...state,
-        visibleMessages: messagesToShow,
-        filteredMessagesCount: filtered,
-      };
-      maybeSortVisibleMessages(filteredState, true);
-
-      return filteredState;
   }
 
   return state;
 }
 /* eslint-enable complexity */
 
+function setVisibleMessages({
+  messagesState,
+  filtersState,
+  prefsState,
+  uiState,
+  forceTimestampSort = false,
+}) {
+  const {
+    messagesById,
+  } = messagesState;
+
+  const messagesToShow = [];
+  const filtered = getDefaultFiltersCounter();
+
+  messagesById.forEach((message, msgId) => {
+    const { visible, cause } = getMessageVisibility(message, {
+      messagesState,
+      filtersState,
+      prefsState,
+      uiState,
+    });
+
+    if (visible) {
+      messagesToShow.push(msgId);
+    } else if (DEFAULT_FILTERS.includes(cause)) {
+      filtered.global = filtered.global + 1;
+      filtered[cause] = filtered[cause] + 1;
+    }
+  });
+
+  const newState = {
+    ...messagesState,
+    visibleMessages: messagesToShow,
+    filteredMessagesCount: filtered,
+  };
+
+  maybeSortVisibleMessages(
+    newState,
+    // Only sort for warningGroups if the feature is enabled
+    prefsState.groupWarnings,
+    forceTimestampSort
+  );
+
+  return newState;
+}
+
 /**
  * Returns the new current group id given the previous current group and the groupsById
  * state property.
@@ -1294,8 +1371,13 @@ function messageCountSinceLastExecutionPoint(state, id) {
  *                                           messages. Default to false, as in some
  *                                           situations we already take care of putting
  *                                           the ids at the right position.
+ * @param {Boolean} timeStampSort: set to true to sort messages by their timestamps.
  */
-function maybeSortVisibleMessages(state, sortWarningGroupMessage = false) {
+function maybeSortVisibleMessages(
+  state,
+  sortWarningGroupMessage = false,
+  timeStampSort = false,
+) {
   // When using log points while replaying, messages can be added out of order
   // with respect to how they originally executed. Use the execution point
   // information in the messages to sort visible messages according to how
@@ -1368,6 +1450,15 @@ function maybeSortVisibleMessages(state, sortWarningGroupMessage = false) {
       return 0;
     });
   }
+
+  if (timeStampSort) {
+    state.visibleMessages.sort((a, b) => {
+      const messageA = state.messagesById.get(a);
+      const messageB = state.messagesById.get(b);
+
+      return messageA.timeStamp < messageB.timeStamp ? -1 : 1;
+    });
+  }
 }
 
 function getLastMessageId(state) {

devtools/client/webconsole/reducers/prefs.js

@@ -5,6 +5,10 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
+const {
+  WARNING_GROUPS_TOGGLE,
+} = require("devtools/client/webconsole/constants");
+
 const PrefState = (overrides) => Object.freeze(Object.assign({
   logLimit: 1000,
   sidebarToggle: false,
@@ -15,6 +19,12 @@ const PrefState = (overrides) => Object.freeze(Object.assign({
 }, overrides));
 
 function prefs(state = PrefState(), action) {
+  if (action.type === WARNING_GROUPS_TOGGLE) {
+    return {
+      ...state,
+      groupWarnings: action.value,
+    };
+  }
   return state;
 }
 

devtools/client/webconsole/test/mochitest/browser.ini

@@ -419,6 +419,7 @@ tags = trackingprotection
 [browser_webconsole_warning_group_content_blocking.js]
 [browser_webconsole_warning_groups_filtering.js]
 [browser_webconsole_warning_groups_outside_console_group.js]
+[browser_webconsole_warning_groups_toggle.js]
 [browser_webconsole_warning_groups.js]
 [browser_webconsole_websocket.js]
 [browser_webconsole_worker_error.js]

devtools/client/webconsole/test/mochitest/browser_webconsole_warning_groups_toggle.js

@@ -0,0 +1,264 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that filtering the console output when there are warning groups works as expected.
+
+"use strict";
+requestLongerTimeout(2);
+
+const {PrefObserver} = require("devtools/client/shared/prefs");
+
+const TEST_FILE =
+  "browser/devtools/client/webconsole/test/mochitest/test-warning-groups.html";
+const TEST_URI = "http://example.org/" + TEST_FILE;
+
+const TRACKER_URL = "http://tracking.example.com/";
+const BLOCKED_URL = TRACKER_URL +
+  "browser/devtools/client/webconsole/test/mochitest/test-image.png";
+const WARNING_GROUP_PREF = "devtools.webconsole.groupWarningMessages";
+
+const {UrlClassifierTestUtils} = ChromeUtils.import("resource://testing-common/UrlClassifierTestUtils.jsm");
+UrlClassifierTestUtils.addTestTrackers();
+registerCleanupFunction(function() {
+  UrlClassifierTestUtils.cleanupTestTrackers();
+});
+
+pushPref("privacy.trackingprotection.enabled", true);
+
+add_task(async function testContentBlockingMessage() {
+  const CONTENT_BLOCKING_GROUP_LABEL = "Content blocked messages";
+
+  // Enable persist log
+  await pushPref("devtools.webconsole.persistlog", true);
+
+  // Start with the warningGroup pref set to false.
+  await pushPref(WARNING_GROUP_PREF, false);
+
+  const hud = await openNewTabAndConsole(TEST_URI);
+
+  info("Log a few content blocking messages and simple ones");
+  let onContentBlockingWarningMessage = waitForMessage(hud, `${BLOCKED_URL}?1`, ".warn");
+  emitStorageAccessBlockedMessage(hud);
+  await onContentBlockingWarningMessage;
+  await logString(hud, "simple message 1");
+
+  onContentBlockingWarningMessage = waitForMessage(hud, `${BLOCKED_URL}?2`, ".warn");
+  emitStorageAccessBlockedMessage(hud);
+  await onContentBlockingWarningMessage;
+
+  onContentBlockingWarningMessage = waitForMessage(hud, `${BLOCKED_URL}?3`, ".warn");
+  emitStorageAccessBlockedMessage(hud);
+  await onContentBlockingWarningMessage;
+
+  checkConsoleOutputForWarningGroup(hud, [
+    `${BLOCKED_URL}?1`,
+    `simple message 1`,
+    `${BLOCKED_URL}?2`,
+    `${BLOCKED_URL}?3`,
+  ]);
+
+  info("Enable the warningGroup feature pref and check warnings were grouped");
+  await toggleWarningGroupPreference(hud);
+  let warningGroupMessage1 =
+    await waitFor(() => findMessage(hud, CONTENT_BLOCKING_GROUP_LABEL));
+  is(warningGroupMessage1.querySelector(".warning-group-badge").textContent, "3",
+    "The badge has the expected text");
+
+  checkConsoleOutputForWarningGroup(hud, [
+    `▶︎⚠ ${CONTENT_BLOCKING_GROUP_LABEL}`,
+    `simple message 1`,
+  ]);
+
+  info("Add a new warning message and check it's placed in the closed group");
+  emitStorageAccessBlockedMessage(hud);
+  await waitForBadgeNumber(warningGroupMessage1, "4");
+
+  info("Re-enable the warningGroup feature pref and check warnings are displayed");
+  await toggleWarningGroupPreference(hud);
+  await waitFor(() => findMessage(hud, `${BLOCKED_URL}?4`));
+
+  // Warning messages are displayed as the expected positions.
+  checkConsoleOutputForWarningGroup(hud, [
+    `${BLOCKED_URL}?1`,
+    `simple message 1`,
+    `${BLOCKED_URL}?2`,
+    `${BLOCKED_URL}?3`,
+    `${BLOCKED_URL}?4`,
+  ]);
+
+  info("Re-disable the warningGroup feature pref");
+  await toggleWarningGroupPreference(hud, false);
+  warningGroupMessage1 =
+    await waitFor(() => findMessage(hud, CONTENT_BLOCKING_GROUP_LABEL));
+
+  checkConsoleOutputForWarningGroup(hud, [
+    `▶︎⚠ ${CONTENT_BLOCKING_GROUP_LABEL}`,
+    `simple message 1`,
+  ]);
+
+  info("Expand the warning group");
+  warningGroupMessage1.querySelector(".arrow").click();
+  await waitFor(() => findMessage(hud, BLOCKED_URL));
+
+  checkConsoleOutputForWarningGroup(hud, [
+    `▼︎⚠ ${CONTENT_BLOCKING_GROUP_LABEL}`,
+    `| ${BLOCKED_URL}?1`,
+    `| ${BLOCKED_URL}?2`,
+    `| ${BLOCKED_URL}?3`,
+    `| ${BLOCKED_URL}?4`,
+    `simple message 1`,
+  ]);
+
+  info("Reload the page and wait for it to be ready");
+  const onDomContentLoaded = BrowserTestUtils.waitForContentEvent(
+    hud.target.tab.linkedBrowser, "DOMContentLoaded", true);
+  ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
+    content.location.reload();
+  });
+  await onDomContentLoaded;
+
+  // Wait for the navigation message to be displayed.
+  await waitFor(() => findMessage(hud, "Navigated to"));
+
+  info("Disable the warningGroup feature pref again");
+  await toggleWarningGroupPreference(hud, false);
+
+  info("Add one warning message and one simple message");
+  await waitFor(() => findMessage(hud, `${BLOCKED_URL}?4`));
+  onContentBlockingWarningMessage = waitForMessage(hud, BLOCKED_URL, ".warn");
+  emitStorageAccessBlockedMessage(hud);
+  await onContentBlockingWarningMessage;
+  await logString(hud, "simple message 2");
+
+  // nothing is grouped.
+  checkConsoleOutputForWarningGroup(hud, [
+    `${BLOCKED_URL}?1`,
+    `simple message 1`,
+    `${BLOCKED_URL}?2`,
+    `${BLOCKED_URL}?3`,
+    `${BLOCKED_URL}?4`,
+    `Navigated to`,
+    `${BLOCKED_URL}?5`,
+    `simple message 2`,
+  ]);
+
+  info("Enable the warningGroup feature pref to check that the group is still expanded");
+  await toggleWarningGroupPreference(hud, false);
+  await waitFor(() => findMessage(hud, CONTENT_BLOCKING_GROUP_LABEL));
+
+  checkConsoleOutputForWarningGroup(hud, [
+    `▼︎⚠ ${CONTENT_BLOCKING_GROUP_LABEL}`,
+    `| ${BLOCKED_URL}?1`,
+    `| ${BLOCKED_URL}?2`,
+    `| ${BLOCKED_URL}?3`,
+    `| ${BLOCKED_URL}?4`,
+    `simple message 1`,
+    `Navigated to`,
+    `| ${BLOCKED_URL}?5`,
+    `simple message 2`,
+  ]);
+
+  info("Add a second warning and check it's placed in the second, closed, group");
+  const onContentBlockingWarningGroupMessage =
+    waitForMessage(hud, CONTENT_BLOCKING_GROUP_LABEL, ".warn");
+  emitStorageAccessBlockedMessage(hud);
+  const warningGroupMessage2 = (await onContentBlockingWarningGroupMessage).node;
+  await waitForBadgeNumber(warningGroupMessage2, "2");
+
+  checkConsoleOutputForWarningGroup(hud, [
+    `▼︎⚠ ${CONTENT_BLOCKING_GROUP_LABEL}`,
+    `| ${BLOCKED_URL}?1`,
+    `| ${BLOCKED_URL}?2`,
+    `| ${BLOCKED_URL}?3`,
+    `| ${BLOCKED_URL}?4`,
+    `simple message 1`,
+    `Navigated to`,
+    `▶︎⚠ ${CONTENT_BLOCKING_GROUP_LABEL}`,
+    `simple message 2`,
+  ]);
+
+  info("Disable the warningGroup pref and check all warning messages are visible");
+  await toggleWarningGroupPreference(hud, false);
+  await waitFor(() => findMessage(hud, `${BLOCKED_URL}?6`));
+
+  checkConsoleOutputForWarningGroup(hud, [
+    `${BLOCKED_URL}?1`,
+    `simple message 1`,
+    `${BLOCKED_URL}?2`,
+    `${BLOCKED_URL}?3`,
+    `${BLOCKED_URL}?4`,
+    `Navigated to`,
+    `${BLOCKED_URL}?5`,
+    `simple message 2`,
+    `${BLOCKED_URL}?6`,
+  ]);
+
+  // Clean the pref for the next tests.
+  Services.prefs.clearUserPref(WARNING_GROUP_PREF);
+});
+
+let cpt = 0;
+/**
+ * Emit a Content Blocking message. This is done by loading an image from an origin
+ * tagged as tracker. The image is loaded with a incremented counter query parameter
+ * each time so we can get the warning message.
+ */
+function emitStorageAccessBlockedMessage(hud) {
+  const url = `${BLOCKED_URL}?${++cpt}`;
+  ContentTask.spawn(gBrowser.selectedBrowser, url, function(innerURL) {
+    content.wrappedJSObject.loadImage(innerURL);
+  });
+}
+
+/**
+ * Log a string from the content page.
+ *
+ * @param {WebConsole} hud
+ * @param {String} str
+ */
+function logString(hud, str) {
+  const onMessage = waitForMessage(hud, str);
+  ContentTask.spawn(gBrowser.selectedBrowser, str, function(arg) {
+    content.console.log(arg);
+  });
+  return onMessage;
+}
+
+function waitForBadgeNumber(message, expectedNumber) {
+  return waitFor(() =>
+    message.querySelector(".warning-group-badge").textContent == expectedNumber);
+}
+
+/**
+ *
+ * @param {WebConsole} hud
+ * @param {Boolean} fromUI: By default, we change the pref by going to the settings panel
+ *                          and clicking the checkbox. If fromUI is set to false, we'll
+ *                          change the pref through Services.prefs to speed-up things.
+ */
+async function toggleWarningGroupPreference(hud, fromUI = true) {
+  if (!fromUI) {
+    await pushPref(WARNING_GROUP_PREF, !Services.prefs.getBoolPref(WARNING_GROUP_PREF));
+    return;
+  }
+
+  info("Open the settings panel");
+  const observer = new PrefObserver("");
+  const toolbox = gDevTools.getToolbox(hud.target);
+  const { panelDoc, panelWin } = await toolbox.selectTool("options");
+
+  info("Change warning preference");
+  const prefChanged = observer.once(WARNING_GROUP_PREF, () => {});
+  const checkbox = panelDoc.getElementById("webconsole-warning-groups");
+
+  // We use executeSoon here to ensure that the element is in view and clickable.
+  checkbox.scrollIntoView();
+  executeSoon(() => EventUtils.synthesizeMouseAtCenter(checkbox, {}, panelWin));
+
+  await prefChanged;
+  observer.destroy();
+
+  // Switch back to the console as it won't update when it is in background
+  info("Go back to console");
+  await toolbox.selectTool("webconsole");
+}

devtools/client/webconsole/test/mochitest/head.js

@@ -1342,6 +1342,9 @@ function checkConsoleOutputForWarningGroup(hud, expectedMessages) {
       }
 
       expectedMessage = expectedMessage.replace("| ", "");
+    } else {
+      is(message.querySelector(".indent").getAttribute("data-indent"),
+        "0", "The message has the expected indent");
     }
 
     ok(message.textContent.trim().includes(expectedMessage.trim()), `Message includes ` +

devtools/client/webconsole/webconsole-wrapper.js

@@ -374,6 +374,11 @@ class WebConsoleWrapper {
         store.dispatch(actions.timestampsToggle(enabled));
       });
 
+      this.prefsObservers.set(PREFS.FEATURES.GROUP_WARNINGS, () => {
+        const enabled = Services.prefs.getBoolPref(PREFS.FEATURES.GROUP_WARNINGS);
+        store.dispatch(actions.warningGroupsToggle(enabled));
+      });
+
       for (const [pref, observer] of this.prefsObservers) {
         Services.prefs.addObserver(pref, observer);
       }