Bug 1559329 - Set "group similar messages" to true by default. r=Honza.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 27 Jun 2019 11:42:15 +0000
changeset 543176 87fa4e9eb13a108440f136703c218a345f5ff8f4
parent 543175 3e211dfbeb71fd8868e9e9d941e1a64e1d760981
child 543177 9c00486078c9290f0750159958aebc1730e0156d
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1559329
milestone69.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 1559329 - Set "group similar messages" to true by default. r=Honza. This was making browser_webconsole_scroll.js fail. After some investigation, I found that the `isInWarningGroup` prop, a function defined in `mapStateToProp` was causing a re-rendering (because we were creating a new function each time), impacting the shouldScrollToBottom behaviour. To fix this, we no longer create a `isInWarningGroup` prop, but directly use the `isMessageInWarningGroup` selector, which was modified to take an array of visible messages instead of the whole state. Differential Revision: https://phabricator.services.mozilla.com/D35016
devtools/client/preferences/devtools-client.js
devtools/client/webconsole/components/Output/ConsoleOutput.js
devtools/client/webconsole/selectors/messages.js
devtools/client/webconsole/test/mochitest/browser_webconsole_trackingprotection_errors.js
--- a/devtools/client/preferences/devtools-client.js
+++ b/devtools/client/preferences/devtools-client.js
@@ -281,18 +281,18 @@ pref("devtools.webconsole.sidebarToggle"
 pref("devtools.webconsole.jsterm.codeMirror", true);
 
 // Enable editor mode in the console.
 pref("devtools.webconsole.input.editor", false);
 
 // Disable the new performance recording panel by default
 pref("devtools.performance.new-panel-enabled", false);
 
-// Enable message grouping in the console, false by default
-pref("devtools.webconsole.groupWarningMessages", false);
+// Enable message grouping in the console, true by default
+pref("devtools.webconsole.groupWarningMessages", true);
 
 // Saved state of the Display content messages checkbox in the browser console.
 pref("devtools.browserconsole.contentMessages", false);
 
 // Enable client-side mapping service for source maps
 pref("devtools.source-map.client-service.enabled", true);
 
 // The number of lines that are displayed in the web console.
--- a/devtools/client/webconsole/components/Output/ConsoleOutput.js
+++ b/devtools/client/webconsole/components/Output/ConsoleOutput.js
@@ -61,17 +61,16 @@ class ConsoleOutput extends Component {
         openContextMenu: PropTypes.func.isRequired,
         sourceMapService: PropTypes.object,
       }),
       dispatch: PropTypes.func.isRequired,
       timestampsVisible: PropTypes.bool,
       messagesPayload: PropTypes.object.isRequired,
       messagesRepeat: PropTypes.object.isRequired,
       warningGroups: PropTypes.object.isRequired,
-      isInWarningGroup: PropTypes.func,
       networkMessagesUpdate: PropTypes.object.isRequired,
       visibleMessages: PropTypes.array.isRequired,
       networkMessageActiveTabId: PropTypes.string.isRequired,
       onFirstMeaningfulPaint: PropTypes.func.isRequired,
       pausedExecutionPoint: PropTypes.any,
     };
   }
 
@@ -176,17 +175,16 @@ class ConsoleOutput extends Component {
     let {
       dispatch,
       visibleMessages,
       messages,
       messagesUi,
       messagesPayload,
       messagesRepeat,
       warningGroups,
-      isInWarningGroup,
       networkMessagesUpdate,
       networkMessageActiveTabId,
       serviceContainer,
       timestampsVisible,
       initialized,
       pausedExecutionPoint,
     } = this.props;
 
@@ -207,18 +205,18 @@ class ConsoleOutput extends Component {
         key: messageId,
         messageId,
         serviceContainer,
         open: messagesUi.includes(messageId),
         payload: messagesPayload.get(messageId),
         timestampsVisible,
         repeat: messagesRepeat[messageId],
         badge: warningGroups.has(messageId) ? warningGroups.get(messageId).length : null,
-        inWarningGroup: isInWarningGroup
-          ? isInWarningGroup(messages.get(messageId))
+        inWarningGroup: warningGroups && warningGroups.size > 0
+          ? isMessageInWarningGroup(messages.get(messageId), visibleMessages)
           : false,
         networkMessageUpdate: networkMessagesUpdate[messageId],
         networkMessageActiveTabId,
         pausedExecutionPoint,
         getMessage: () => messages.get(messageId),
         isPaused: !!pausedMessage && pausedMessage.id == messageId,
         maybeScrollToBottom: this.maybeScrollToBottom,
       }));
@@ -255,18 +253,15 @@ function mapStateToProps(state, props) {
     initialized: state.ui.initialized,
     pausedExecutionPoint: getPausedExecutionPoint(state),
     messages: getAllMessagesById(state),
     visibleMessages: getVisibleMessages(state),
     messagesUi: getAllMessagesUiById(state),
     messagesPayload: getAllMessagesPayloadById(state),
     messagesRepeat: getAllRepeatById(state),
     warningGroups: getAllWarningGroupsById(state),
-    isInWarningGroup: state.prefs.groupWarnings
-      ? message => isMessageInWarningGroup(state, message)
-      : null,
     networkMessagesUpdate: getAllNetworkMessagesUpdateById(state),
     timestampsVisible: state.ui.timestampsVisible,
     networkMessageActiveTabId: state.ui.networkMessageActiveTabId,
   };
 }
 
 module.exports = connect(mapStateToProps)(ConsoleOutput);
--- a/devtools/client/webconsole/selectors/messages.js
+++ b/devtools/client/webconsole/selectors/messages.js
@@ -54,22 +54,22 @@ function getGroupsById(state) {
 function getPausedExecutionPoint(state) {
   return state.messages.pausedExecutionPoint;
 }
 
 function getAllWarningGroupsById(state) {
   return state.messages.warningGroupsById;
 }
 
-function isMessageInWarningGroup(state, message) {
+function isMessageInWarningGroup(message, visibleMessages = []) {
   if (!getWarningGroupType(message)) {
     return false;
   }
 
-  return getVisibleMessages(state).includes(getParentWarningGroupMessageId(message));
+  return visibleMessages.includes(getParentWarningGroupMessageId(message));
 }
 
 module.exports = {
   getAllGroupsById,
   getAllWarningGroupsById,
   getAllMessagesById,
   getAllMessagesPayloadById,
   getAllMessagesUiById,
--- a/devtools/client/webconsole/test/mochitest/browser_webconsole_trackingprotection_errors.js
+++ b/devtools/client/webconsole/test/mochitest/browser_webconsole_trackingprotection_errors.js
@@ -28,16 +28,18 @@ const COOKIE_BEHAVIORS = {
 };
 
 const {UrlClassifierTestUtils} = ChromeUtils.import("resource://testing-common/UrlClassifierTestUtils.jsm");
 
 registerCleanupFunction(function() {
   UrlClassifierTestUtils.cleanupTestTrackers();
 });
 
+pushPref("devtools.webconsole.groupWarningMessages", false);
+
 add_task(async function testContentBlockingMessage() {
   await UrlClassifierTestUtils.addTestTrackers();
 
   await pushPref("privacy.trackingprotection.enabled", true);
   const hud = await openNewTabAndConsole(TRACKER_URL + TEST_FILE);
 
   info("Test content blocking message");
   const message = await waitFor(() => findMessage(hud,
@@ -85,18 +87,17 @@ add_task(async function testAllCookieBlo
   info("Test all cookies blocked message");
   // We change the pref and open a new window to ensure it will be taken into account.
   await pushPref(COOKIE_BEHAVIOR_PREF, COOKIE_BEHAVIORS.REJECT);
   const {hud, win} = await openNewWindowAndConsole(TEST_URI);
 
   const message = await waitFor(() => findMessage(hud,
     `Request to access cookie or storage on ${BLOCKED_URL} was blocked because we are ` +
     `blocking all storage access requests`));
-  await testLearnMoreClickOpenNewTab(message,
-    getStorageErrorUrl("CookieBlockedAll"));
+  await testLearnMoreClickOpenNewTab(message, getStorageErrorUrl("CookieBlockedAll"));
   win.close();
 });
 
 add_task(async function testTrackerCookieBlockedMessage() {
   info("Test tracker cookie blocked message");
   // We change the pref and open a new window to ensure it will be taken into account.
   await pushPref(COOKIE_BEHAVIOR_PREF, COOKIE_BEHAVIORS.REJECT_TRACKER);
   const {hud, win} = await openNewWindowAndConsole(TEST_URI);