Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Add regex filtering for network monitor

VERIFIED FIXED in Firefox 55

Status

()

Firefox
Developer Tools: Netmonitor
VERIFIED FIXED
4 months ago
3 months ago

People

(Reporter: ntim, Assigned: Ruturaj Vartak)

Tracking

(Blocks: 1 bug, {dev-doc-complete, good-first-bug})

unspecified
Firefox 55
dev-doc-complete, good-first-bug
Points:
---

Firefox Tracking Flags

(firefox55 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 months ago
Would probably need an UI like:

[x] Use regular expressions
(Reporter)

Updated

4 months ago
Blocks: 1093269
(Reporter)

Updated

4 months ago
No longer blocks: 859047
(Reporter)

Comment 1

3 months ago
I think we can add a new simple regexp flag that accepts a regular expression that will filter through URLs.
Keywords: good-first-bug
(Assignee)

Comment 2

3 months ago
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)
(Reporter)

Comment 3

3 months ago
(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)
(Assignee)

Comment 4

3 months ago
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)

Updated

3 months ago
Assignee: nobody → ruturaj
(Reporter)

Comment 5

3 months ago
(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)
(Assignee)

Comment 6

3 months ago
Created attachment 8862759 [details] [diff] [review]
fix-1354495.patch

- Impelemnted "regexp" prefix filter
- Added a test case
Attachment #8862759 - Flags: review?(ntim.bugs)
(Reporter)

Comment 7

3 months ago
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 ?
(Reporter)

Updated

3 months ago
Flags: needinfo?(ruturaj)
(Assignee)

Comment 8

3 months ago
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)
(Reporter)

Comment 9

3 months ago
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+
(Assignee)

Comment 10

3 months ago
Created attachment 8863091 [details] [diff] [review]
fix-1354495-2.patch

- Fixed the nit of `break` after return statement.
Attachment #8862759 - Attachment is obsolete: true
Attachment #8863091 - Flags: review?(ntim.bugs)
(Reporter)

Updated

3 months ago
Attachment #8863091 - Flags: review?(ntim.bugs) → review+
(Reporter)

Updated

3 months ago
Keywords: checkin-needed
(Assignee)

Comment 11

3 months ago
Thanks Tim.

Comment 12

3 months ago
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
(Reporter)

Updated

3 months ago
Keywords: dev-doc-needed
https://hg.mozilla.org/mozilla-central/rev/5bc16cdc9f85
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox55: --- → fixed
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]
(Reporter)

Updated

3 months ago
Status: RESOLVED → VERIFIED
status-firefox55: fixed → 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)
(Assignee)

Comment 16

3 months ago
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
Keywords: dev-doc-needed → dev-doc-complete
See Also: → bug 1362030
(Assignee)

Comment 18

3 months ago
That's fine Sebastian, Thanks a lot.
You need to log in before you can comment on or make changes to this bug.