Closed
Bug 1354495
Opened 8 years ago
Closed 8 years ago
Add regex filtering for network monitor
Categories
(DevTools :: Netmonitor, enhancement)
DevTools
Netmonitor
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)
2.88 KB,
patch
|
ntim
:
review+
|
Details | Diff | Splinter Review |
Would probably need an UI like: [x] Use regular expressions
Reporter | ||
Updated•8 years ago
|
Blocks: netmonitor-filtering
Reporter | ||
Comment 1•8 years 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•8 years 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•8 years 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•8 years 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•8 years ago
|
Assignee: nobody → ruturaj
Reporter | ||
Comment 5•8 years 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•8 years ago
|
||
- Impelemnted "regexp" prefix filter - Added a test case
Attachment #8862759 -
Flags: review?(ntim.bugs)
Reporter | ||
Comment 7•8 years 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•8 years ago
|
Flags: needinfo?(ruturaj)
Assignee | ||
Comment 8•8 years 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•8 years 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•8 years ago
|
||
- Fixed the nit of `break` after return statement.
Attachment #8862759 -
Attachment is obsolete: true
Attachment #8863091 -
Flags: review?(ntim.bugs)
Reporter | ||
Updated•8 years ago
|
Attachment #8863091 -
Flags: review?(ntim.bugs) → review+
Reporter | ||
Updated•8 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 11•8 years ago
|
||
Thanks Tim.
Comment 12•8 years 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•8 years ago
|
Keywords: dev-doc-needed
Comment 13•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5bc16cdc9f85
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment 14•8 years ago
|
||
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•8 years ago
|
Status: RESOLVED → VERIFIED
Comment 15•8 years ago
|
||
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•8 years 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)
Comment 17•8 years ago
|
||
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
Assignee | ||
Comment 18•8 years ago
|
||
That's fine Sebastian, Thanks a lot.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•