1354495 - Add regex filtering for network monitor

Status: VERIFIED FIXED

(DevTools :: Netmonitor, enhancement)

Description

[Tim Nguyen :ntim]

Would probably need an UI like:

[x] Use regular expressions

Comment 1

[Tim Nguyen :ntim]

I think we can add a new simple regexp flag that accepts a regular expression that will filter through URLs.

Comment 2

[Ruturaj Vartak]

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) ?

Comment 3

[Tim Nguyen :ntim]

(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.

Comment 4

[Ruturaj Vartak]

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?

Comment 5

[Tim Nguyen :ntim]

(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)

Comment 6

[Ruturaj Vartak]

Attachment: fix-1354495.patch

- Impelemnted "regexp" prefix filter
- Added a test case

Comment 7

[Tim Nguyen :ntim]

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 ?

Comment 8

[Ruturaj Vartak]

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).

Comment 9

[Tim Nguyen :ntim]

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.

Comment 10

[Ruturaj Vartak]

Attachment: fix-1354495-2.patch

- Fixed the nit of `break` after return statement.

Comment 11

[Ruturaj Vartak]

Thanks Tim.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bc16cdc9f85
Add regex filtering for network monitor. r=ntim

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5bc16cdc9f85

Comment 14

[Nazir Ahmed Sabbir [:NaSb]]

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

Comment 15

[Sebastian Zartner [:sebo]]

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

Comment 16

[Ruturaj Vartak]

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.

Comment 17

[Sebastian Zartner [:sebo]]

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

Comment 18

[Ruturaj Vartak]

That's fine Sebastian, Thanks a lot.