Skip to content
This repository was archived by the owner on Jul 30, 2026. It is now read-only.

fix: wrap onRequestFinished to use promises - #250

Merged
Rob--W merged 1 commit into
mozilla:masterfrom
dgp1130:get-content
Dec 10, 2020
Merged

fix: wrap onRequestFinished to use promises#250
Rob--W merged 1 commit into
mozilla:masterfrom
dgp1130:get-content

Conversation

@dgp1130

@dgp1130 dgp1130 commented Dec 7, 2020

Copy link
Copy Markdown
Contributor

Fixes #249.

This updates browser.devtools.network.onRequestFinished to emit an
object with a promisified getContent() property. This brings the
polyfill implementation in line with Firefox's implementation, although
documentation is still inaccurate at the moment.

Also updates some out of date documentation with makeCallback().

I tested this in my own Chrome extension and it appears to work as expected.

@dgp1130

dgp1130 commented Dec 7, 2020

Copy link
Copy Markdown
Contributor Author

CI seems to be having a versioning error with Chrome that seems to be unrelated to this PR?

Comment thread src/browser-polyfill.js Outdated
Comment thread src/browser-polyfill.js
Comment thread test/test-onRequestFinished.js
@Rob--W

Rob--W commented Dec 8, 2020

Copy link
Copy Markdown
Member

CI seems to be having a versioning error with Chrome that seems to be unrelated to this PR?

I'll take care of that in #251

Comment thread test/test-onRequestFinished.js
Comment thread test/test-onRequestFinished.js Outdated
Comment thread test/test-onRequestFinished.js Outdated

@Rob--W Rob--W left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks. I'll merge it after the other PR that unbreaks the build is merged.

@Rob--W

Rob--W commented Dec 10, 2020

Copy link
Copy Markdown
Member

The other PR got approved and merged. Could you rebase and update the PR so we can get a green CI build?

Fixes #249.

This updates `browser.devtools.network.onRequestFinished` to emit an
object with a promisified `getContent()` property. This brings the
polyfill implementation in line with Firefox's implementation, although
MDN documentation is still inaccurate at the moment.

Also updates some out of date documentation with `makeCallback()` and
`wrapAsyncFunction()`.
@dgp1130

dgp1130 commented Dec 10, 2020

Copy link
Copy Markdown
Contributor Author

Rebased and got a green CI, thanks for fixing that. Should be ready to merge.

@Rob--W

Rob--W commented Dec 10, 2020

Copy link
Copy Markdown
Member

Thanks for the report and patch!

@Rob--W
Rob--W merged commit 716c90b into mozilla:master Dec 10, 2020
@dgp1130
dgp1130 deleted the get-content branch December 10, 2020 22:26
@caitmuenster

Copy link
Copy Markdown

Thanks so much for the patch, @dgp1130! Your contribution has been added to our recognition wiki.

Take care!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

browser.devtools.network.onRequestFinished does not promisify getContent()

3 participants