Bug 1552109 - Fix filter count when text filtering is on. r=bgrins.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 16 May 2019 13:12:06 +0000
changeset 532904 8e5c0606d1555b052c98ab5d01dc6076b4453039
parent 532903 20457619f22b5c08a94f5423c0388f84312f6a43
child 532905 14d8005e2dd100d19c7ce8cdf0e10cc4ca3fc2a7
push id11276
push userrgurzau@mozilla.com
push dateMon, 20 May 2019 13:11:24 +0000
treeherdermozilla-beta@847755a7c325 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1552109
milestone68.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 1552109 - Fix filter count when text filtering is on. r=bgrins. The counter was erroneous because we were checking that messages were validating the text input before checking if they should be visible based on their category. Sadly, mocha tests were asserting that we had a bug, so we fixed them in this patch. Differential Revision: https://phabricator.services.mozilla.com/D31402
devtools/client/webconsole/reducers/messages.js
devtools/client/webconsole/test/components/filter-bar.test.js
devtools/client/webconsole/test/store/hidden-messages.test.js
--- a/devtools/client/webconsole/reducers/messages.js
+++ b/devtools/client/webconsole/reducers/messages.js
@@ -842,23 +842,16 @@ function getMessageVisibility(message, {
   // Some messages can't be filtered out (e.g. groups).
   // So, always return visible: true for those.
   if (isUnfilterable(message)) {
     return {
       visible: true,
     };
   }
 
-  if (!passSearchFilters(message, filtersState)) {
-    return {
-      visible: false,
-      cause: FILTERS.TEXT,
-    };
-  }
-
   // Let's check all level filters (error, warn, log, …) and return visible: false
   // and the message level as a cause if the function returns false.
   if (!passLevelFilters(message, filtersState)) {
     return {
       visible: false,
       cause: message.level,
     };
   }
@@ -879,16 +872,25 @@ function getMessageVisibility(message, {
 
   if (!passXhrFilter(message, filtersState)) {
     return {
       visible: false,
       cause: FILTERS.NETXHR,
     };
   }
 
+  // This should always be the last check, or we might report that a message was hidden
+  // because of text search, while it may be hidden because its category is disabled.
+  if (!passSearchFilters(message, filtersState)) {
+    return {
+      visible: false,
+      cause: FILTERS.TEXT,
+    };
+  }
+
   return {
     visible: true,
   };
 }
 
 function isUnfilterable(message) {
   return [
     MESSAGE_TYPE.COMMAND,
--- a/devtools/client/webconsole/test/components/filter-bar.test.js
+++ b/devtools/client/webconsole/test/components/filter-bar.test.js
@@ -134,21 +134,16 @@ describe("FilterBar component:", () => {
       "console.error('error message');",
       "console.log('foobar', 'test')",
       "console.info('info message');",
       "console.warn('danger, will robinson!')",
       "console.debug('debug message');",
       "console.error('error message');",
     ]);
 
-    store.dispatch(actions.filterToggle(FILTERS.ERROR));
-    store.dispatch(actions.filterToggle(FILTERS.WARN));
-    store.dispatch(actions.filterToggle(FILTERS.LOG));
-    store.dispatch(actions.filterToggle(FILTERS.INFO));
-    store.dispatch(actions.filterToggle(FILTERS.DEBUG));
     store.dispatch(actions.filterTextSet("qwerty"));
 
     const wrapper = mount(Provider({store}, getFilterBar()));
     const message = wrapper.find(".filter-message-text");
 
     expect(message.prop("title")).toBe("text: 10");
   });
 
--- a/devtools/client/webconsole/test/store/hidden-messages.test.js
+++ b/devtools/client/webconsole/test/store/hidden-messages.test.js
@@ -26,26 +26,56 @@ describe("Filtering - Hidden messages", 
   });
 
   it("has the expected numbers", () => {
     const counter = getFilteredMessagesCount(store.getState());
     expect(counter).toEqual(BASIC_TEST_CASE_FILTERED_MESSAGE_COUNT);
   });
 
   it("has the expected numbers when there is a text search", () => {
+    // "info" is disabled and the filter input only matches a warning message.
+    store.dispatch(actions.filtersClear());
+    store.dispatch(actions.filterToggle(FILTERS.INFO));
     store.dispatch(actions.filterTextSet("danger, will robinson!"));
+
     let counter = getFilteredMessagesCount(store.getState());
     expect(counter).toEqual({
       [FILTERS.ERROR]: 0,
-      // The warning message matches the text search.
-      [FILTERS.WARN]: 1,
+      [FILTERS.WARN]: 0,
+      [FILTERS.LOG]: 0,
+      [FILTERS.INFO]: 1,
+      [FILTERS.DEBUG]: 0,
+      [FILTERS.TEXT]: 9,
+      global: 10,
+    });
+
+    // Numbers update if the text search is cleared.
+    store.dispatch(actions.filterTextSet(""));
+    counter = getFilteredMessagesCount(store.getState());
+    expect(counter).toEqual({
+      [FILTERS.ERROR]: 0,
+      [FILTERS.WARN]: 0,
       [FILTERS.LOG]: 0,
-      [FILTERS.INFO]: 0,
+      [FILTERS.INFO]: 1,
       [FILTERS.DEBUG]: 0,
-      [FILTERS.TEXT]: 10,
+      [FILTERS.TEXT]: 0,
+      global: 1,
+    });
+  });
+
+  it("has the expected numbers when there's a text search on disabled categories", () => {
+    store.dispatch(actions.filterTextSet("danger, will robinson!"));
+    let counter = getFilteredMessagesCount(store.getState());
+    expect(counter).toEqual({
+      [FILTERS.ERROR]: 3,
+      [FILTERS.WARN]: 1,
+      [FILTERS.LOG]: 5,
+      [FILTERS.INFO]: 1,
+      [FILTERS.DEBUG]: 1,
+      [FILTERS.TEXT]: 0,
       global: 11,
     });
 
     // Numbers update if the text search is cleared.
     store.dispatch(actions.filterTextSet(""));
     counter = getFilteredMessagesCount(store.getState());
     expect(counter).toEqual(BASIC_TEST_CASE_FILTERED_MESSAGE_COUNT);
   });