Closed Bug 1354495 Opened 7 years ago Closed 7 years ago

Add regex filtering for network monitor

Categories

(DevTools :: Netmonitor, enhancement)

enhancement
Not set
normal

Tracking

(firefox55 verified)

VERIFIED FIXED
Firefox 55
Tracking Status
firefox55 --- verified

People

(Reporter: ntim, Assigned: ruturaj)

References

Details

(Keywords: dev-doc-complete, good-first-bug)

Attachments

(1 file, 1 obsolete file)

Would probably need an UI like:

[x] Use regular expressions
No longer blocks: 859047
I think we can add a new simple regexp flag that accepts a regular expression that will filter through URLs.
Keywords: good-first-bug
Hey Tim,
I was thinking of picking this up. Thought should I work on this with bug#1354504 ? or bug#1354504 is a bigger one to solve ?

Also, should I work on the standalone netmonitor (devtools-core its called I think) or the standard one (that runs with firefox) ?
Flags: needinfo?(ntim.bugs)
(In reply to Ruturaj Vartak from comment #2)
> Hey Tim,
> I was thinking of picking this up. Thought should I work on this with
> bug#1354504 ? or bug#1354504 is a bigger one to solve ?

Thanks for picking this up!
bug#1354504 is a bigger task than this bug. Feel free to pick what you feel more comfortable with first :)

> Also, should I work on the standalone netmonitor (devtools-core its called I
> think) or the standard one (that runs with firefox) ?

It should be the same really, you can choose what's more convenient for you.
Flags: needinfo?(ntim.bugs)
Thanks Tim,

Instead  of a checkbox, i wonder if a prefix based search like "pattern:{pattern}" is preferred? This would be similar to column based search.
Im assuming things might change a little with autocomplete implementation.

Also, which set of columns should i include for pattern?
Flags: needinfo?(ntim.bugs)
Assignee: nobody → ruturaj
(In reply to Ruturaj Vartak from comment #4)
> Thanks Tim,
> 
> Instead  of a checkbox, i wonder if a prefix based search like
> "pattern:{pattern}" is preferred? This would be similar to column based
> search.
> Im assuming things might change a little with autocomplete implementation.
Maybe "regexp:{regexp}" is better ?

> Also, which set of columns should i include for pattern?

Only the URL (like for plain text filtering)
Flags: needinfo?(ntim.bugs)
Attached patch fix-1354495.patch (obsolete) — Splinter Review
- Impelemnted "regexp" prefix filter
- Added a test case
Attachment #8862759 - Flags: review?(ntim.bugs)
Comment on attachment 8862759 [details] [diff] [review]
fix-1354495.patch

Review of attachment 8862759 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/netmonitor/src/utils/filter-text-utils.js
@@ +91,5 @@
>  function processFlagFilter(type, value) {
>    switch (type) {
> +    case "regexp":
> +      /* eslint-disable no-unreachable */
> +      return value;

Why is this needed ?
Flags: needinfo?(ruturaj)
by default the value is returned as toLowerCase(), if somebody adds a regex, that would be converted to lower - which would go against the expectation (typically for informed users using a pattern).
Flags: needinfo?(ruturaj)
Comment on attachment 8862759 [details] [diff] [review]
fix-1354495.patch

Review of attachment 8862759 [details] [diff] [review]:
-----------------------------------------------------------------

Oh right, I was confused about how Splinter displayed the diff.

Works well for me, thanks!

::: devtools/client/netmonitor/src/utils/filter-text-utils.js
@@ +92,5 @@
>    switch (type) {
> +    case "regexp":
> +      /* eslint-disable no-unreachable */
> +      return value;
> +      break;

nit: remove the break statement and the eslint-disable comment.
Attachment #8862759 - Flags: review?(ntim.bugs) → review+
- Fixed the nit of `break` after return statement.
Attachment #8862759 - Attachment is obsolete: true
Attachment #8863091 - Flags: review?(ntim.bugs)
Attachment #8863091 - Flags: review?(ntim.bugs) → review+
Keywords: checkin-needed
Thanks Tim.
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bc16cdc9f85
Add regex filtering for network monitor. r=ntim
Keywords: checkin-needed
Keywords: dev-doc-needed
https://hg.mozilla.org/mozilla-central/rev/5bc16cdc9f85
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
I reproduced this issue using Nightly 55.0a1 (2017-04-07), Build ID: 20170407100252 on Linux x64.

I can confirm this issue is now verified as fixed on Latest Firefox Nightly 55.0a1 (2017-04-30) (64-bit)

Build ID: 20170430100638
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
OS: Linux 4.4.0-72-generic; Elementary OS 0.4
QA Whiteboard: [testday-20170428]
Status: RESOLVED → VERIFIED
I've added the related description of the regexp filtering to https://developer.mozilla.org/en-US/docs/Tools/Network_Monitor#Filtering_by_properties and added a developer release note to https://developer.mozilla.org/en-US/Firefox/Releases/55#Developer_Tools.

Ruturaj, can you please have a look at the documentation and let me know whether it's ok?

Sebastian
Flags: needinfo?(ruturaj)
Hey Sebastian,

The regexp matches only the URL (the Entire URL http://domain.com/path/to/file). So perhaps we could have this

`Show the resources that match the given regular expression with its URL.` or something similar.

Thanks.
Flags: needinfo?(ruturaj)
Right, thanks for noting! I've simply forgot to mention that. :-)
So, I have changed the sentence now to "Show the resources having a URL that matches the given regular expression.".

Sebastian
See Also: → 1362030
That's fine Sebastian, Thanks a lot.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: