Closed
Bug 1354508
Opened 8 years ago
Closed 8 years ago
Add filter option for network requests checking for a specific response header
Categories
(DevTools :: Netmonitor, enhancement)
Tracking
(firefox55 verified)
VERIFIED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | verified |
People
(Reporter: sebo, Assigned: vkatsikaros)
References
Details
(Keywords: dev-doc-complete, good-first-bug)
Attachments
(2 files)
The filter field within the Network Monitor should support a filter option that checks the requests for a specific HTTP header in the response.
The Chrome DevTools provide a 'has-response-header' filter for this purpose.
Sebastian
Reporter | ||
Updated•8 years ago
|
Updated•8 years ago
|
Keywords: good-first-bug
Assignee | ||
Comment 1•8 years ago
|
||
I think I know what to tweak for this bug. Could I have this assigned?
Should I use the same filter name, ie 'has-response-header'?
Flags: needinfo?(ntim.bugs)
Updated•8 years ago
|
Assignee: nobody → vkatsikaros
Flags: needinfo?(ntim.bugs)
Comment 2•8 years ago
|
||
(In reply to Vangelis Katsikaros from comment #1)
> I think I know what to tweak for this bug. Could I have this assigned?
I've given you permission to assign yourself to bugs, so you can do it on any bug next time :)
> Should I use the same filter name, ie 'has-response-header'?
Yes, has-response-header.
Assignee | ||
Comment 3•8 years ago
|
||
Thanks Tim!
Assignee | ||
Comment 4•8 years ago
|
||
Comment hidden (mozreview-request) |
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8858114 [details]
Bug 1354508 - Add filter option for network requests checking for a specific response header.
https://reviewboard.mozilla.org/r/130084/#review132708
Looks good generally, thanks for working on this!
I have a few questions before granting a ship-it.
::: devtools/client/netmonitor/src/utils/filter-text-utils.js:133
(Diff revision 1)
> break;
> case "remote-ip":
> match = `${item.remoteAddress}:${item.remotePort}`.toLowerCase().includes(value);
> break;
> + case "has-response-header":
> + if (typeof item.responseHeaders === "object") {
Does item.hasOwnProperty("responseHeaders") work instead ? or does that omit some cases ?
::: devtools/client/netmonitor/src/utils/filter-text-utils.js:134
(Diff revision 1)
> + let responseHeaders = item.responseHeaders.headers;
> + let r = responseHeaders.findIndex((e) => {
> + return e.name.toLowerCase() === value;
> + });
You can simplify this a bit:
let { headers } = item.responseHeaders;
match = headers.findIndex(h => h.name.toLowerCase() === value) > -1;
::: devtools/client/netmonitor/src/utils/filter-text-utils.js:134
(Diff revision 1)
> + let responseHeaders = item.responseHeaders.headers;
> + let r = responseHeaders.findIndex((e) => {
> + return e.name.toLowerCase() === value;
> + });
You can simplify this a bit:
let { headers } = item.responseHeaders;
match = headers.findIndex(h => h.name.toLowerCase() === value) > 0;
::: devtools/client/netmonitor/src/utils/filter-text-utils.js:138
(Diff revision 1)
> + if (typeof item.responseHeaders === "object") {
> + let responseHeaders = item.responseHeaders.headers;
> + let r = responseHeaders.findIndex((e) => {
> + return e.name.toLowerCase() === value;
> + });
> + match = r > 0;
Shouldn't that be `> -1` instead of `> 0`? Otherwise, if the first index is matched, it will return false.
Or am I missing something here (eg. the first item has a special meaning?) ?
Assignee | ||
Comment 7•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8858114 [details]
Bug 1354508 - Add filter option for network requests checking for a specific response header.
https://reviewboard.mozilla.org/r/130084/#review132708
> Does item.hasOwnProperty("responseHeaders") work instead ? or does that omit some cases ?
This twisted my brain a bit, and I apologize for missing something obvious.
With:
```
@@ -128,6 +129,19 @@ function isFlagFilterMatch(item, { type,
case "remote-ip":
match = `${item.remoteAddress}:${item.remotePort}`.toLowerCase().includes(value);
break;
+ case "has-response-header":
+console.log( typeof item);
+console.log( typeof item.responseHeaders);
+console.log( typeof item.responseHeaders.headers);
+console.log( typeof item.responseHeaders === "object");
+console.log(item.hasOwnProperty("responseHeaders"));
+ if (item.hasOwnProperty("responseHeaders")) {
+ let { headers } = item.responseHeaders;
+ match = headers.findIndex(h => h.name.toLowerCase() === value) > -1;
+ } else {
+ match = false;
+ }
+ break;
```
I get in the browser console (if I filter the network panel) the output:
```
object filter-text-utils.js:133:1
object filter-text-utils.js:134:1
object filter-text-utils.js:135:1
true filter-text-utils.js:136:1
false filter-text-utils.js:137:1
```
with `item.hasOwnProperty("responseHeaders")` giving `false` whereas the key exists in the object.
I initially decided to workaround this, but it seems there is a lesson to be learned here, so I was wondering if I could get some feedback.
> You can simplify this a bit:
>
> let { headers } = item.responseHeaders;
> match = headers.findIndex(h => h.name.toLowerCase() === value) > -1;
Neater
> Shouldn't that be `> -1` instead of `> 0`? Otherwise, if the first index is matched, it will return false.
>
> Or am I missing something here (eg. the first item has a special meaning?) ?
Ah! I meant to type ">= 0" (but didn't) :/ However, your suggestion hints more cleanly to the return value of findIndex
Comment 8•8 years ago
|
||
mozreview-review |
Comment on attachment 8858114 [details]
Bug 1354508 - Add filter option for network requests checking for a specific response header.
https://reviewboard.mozilla.org/r/130084/#review132916
r+ with my 2 comments addressed.
Attachment #8858114 -
Flags: review?(ntim.bugs) → review+
Comment 9•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8858114 [details]
Bug 1354508 - Add filter option for network requests checking for a specific response header.
https://reviewboard.mozilla.org/r/130084/#review132708
> This twisted my brain a bit, and I apologize for missing something obvious.
>
> With:
>
> ```
> @@ -128,6 +129,19 @@ function isFlagFilterMatch(item, { type,
> case "remote-ip":
> match = `${item.remoteAddress}:${item.remotePort}`.toLowerCase().includes(value);
> break;
> + case "has-response-header":
> +console.log( typeof item);
> +console.log( typeof item.responseHeaders);
> +console.log( typeof item.responseHeaders.headers);
> +console.log( typeof item.responseHeaders === "object");
> +console.log(item.hasOwnProperty("responseHeaders"));
> + if (item.hasOwnProperty("responseHeaders")) {
> + let { headers } = item.responseHeaders;
> + match = headers.findIndex(h => h.name.toLowerCase() === value) > -1;
> + } else {
> + match = false;
> + }
> + break;
> ```
> I get in the browser console (if I filter the network panel) the output:
> ```
> object filter-text-utils.js:133:1
> object filter-text-utils.js:134:1
> object filter-text-utils.js:135:1
> true filter-text-utils.js:136:1
> false filter-text-utils.js:137:1
> ```
> with `item.hasOwnProperty("responseHeaders")` giving `false` whereas the key exists in the object.
>
> I initially decided to workaround this, but it seems there is a lesson to be learned here, so I was wondering if I could get some feedback.
Sounds fine then, dropping this issue.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•8 years ago
|
||
Tim, as far as the process is concerned, do I also need to trigger a try, on top of the push to mozreview?
Also, I would be curious to find an explanation for the hasOwnProperty behavior in https://bugzilla.mozilla.org/show_bug.cgi?id=1354508#c7 , I really can't figure it out :/
Flags: needinfo?(ntim.bugs)
Comment 12•8 years ago
|
||
(In reply to Vangelis Katsikaros from comment #11)
> Tim, as far as the process is concerned, do I also need to trigger a try, on
> top of the push to mozreview?
This patch should be safe enough for tests, so no need for a try push.
> Also, I would be curious to find an explanation for the hasOwnProperty
> behavior in https://bugzilla.mozilla.org/show_bug.cgi?id=1354508#c7 , I
> really can't figure it out :/
Not sure, probably because responseHeaders is not defined as a property but as a getter or something else.
Flags: needinfo?(ntim.bugs)
Comment 13•8 years ago
|
||
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6a9ce1980dcc
Add filter option for network requests checking for a specific response header. r=ntim
![]() |
||
Comment 14•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment 15•8 years ago
|
||
I have reproduced this bug with Nightly 55.0a1 (2017-04-07) on Windows 10 , 64 Bit !
This bug's fix is Verified with latest Nightly !
Build ID 20170427030231
User Agent Mozilla/5.0 (Windows NT 10.0; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0
[bugday-20170426]
Updated•8 years ago
|
Reporter | ||
Comment 16•8 years ago
|
||
Added description of the filter to https://developer.mozilla.org/en-US/docs/Tools/Network_Monitor#Filtering_by_properties and a developer release note to https://developer.mozilla.org/en-US/Firefox/Releases/55#Developer_Tools.
Vangelis, can you please have a look at the documentation and let me know whether it's ok?
Sebastian
Flags: needinfo?(vkatsikaros)
Reporter | ||
Comment 18•8 years ago
|
||
Great! Thanks for the fast response!
Sebastian
Keywords: dev-doc-needed → dev-doc-complete
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•