support for chrome devtools _initiator attributes 'type', 'url' and 'lineNumber' #26
Conversation
deviantintegral left a comment
deviantintegral left a comment There was a problem hiding this comment.
This looks great! Thanks for contributing.
There's one typo that needs to be fixed for sure. Otherwise, what do you think about adding a fixture that includes the initiator property? That would automatically be tested by https://github.com/deviantintegral/har/blob/master/tests/src/Unit/HarTest.php for the basic import / export case.
| * @Serializer\Type("string") | ||
| * | ||
| * parser: | ||
| * Chrome's HTML parser. |
There was a problem hiding this comment.
Is this truly just Chrome, or any Blink-derived browser?
There was a problem hiding this comment.
The expected values are from https://developers.google.com/web/tools/chrome-devtools/network/reference#requests
You are absolutely correct, all blink based browsers with devtools would use these values.
tests/src/Unit/EntryTest.php Outdated
| public function testHasInitiator() | ||
| { | ||
| // Initialize a repository of HAR files, with IDs being the file names. | ||
| $repository = |
There was a problem hiding this comment.
typo here? This looks like it's double-assigning the next variable.
There was a problem hiding this comment.
indeed, the thought of moving stuff to setUp wasn't finished when I committed ;-)
| Looks like I didn't have the option to run CI on forks turned on. Your next commit should trigger them. |
beatchristen force-pushed the chrome-devtools-_initiator-attributes branch from 09aa3d7 to 6a0df89 Compare 
tests/src/Unit/EntryTest.php Outdated
| public function testHasInitiator() | ||
| { | ||
| // Initialize a repository of HAR files, with IDs being the file names. | ||
| $repository = |
There was a problem hiding this comment.
indeed, the thought of moving stuff to setUp wasn't finished when I committed ;-)
| * @Serializer\Type("string") | ||
| * | ||
| * parser: | ||
| * Chrome's HTML parser. |
There was a problem hiding this comment.
The expected values are from https://developers.google.com/web/tools/chrome-devtools/network/reference#requests
You are absolutely correct, all blink based browsers with devtools would use these values.
| return $this; | ||
| } | ||
| | ||
| public function getUrl(): ?\Psr\Http\Message\UriInterface |
There was a problem hiding this comment.
added nullability infos here.
That's what I did at the beginning, but and putting JUST Potentially, refactoring the current Serializer which is hardcoded in several places like That's however a topic for another issue, I think... |
| Agreed on a new issue or pull request - I think that approach is a good idea, because it could also let us support future versions of HAR should they ever happen. |
2 participants
Hi @deviantintegral
I'm interested also in the non-standard attributes that chrome devtools exports since around chrome 67 or so.
All new APIs should be covered by tests.