Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[webview] prevent potential double resolution in XMLHttpRequest handling #22404

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

0xlau
Copy link
Contributor

@0xlau 0xlau commented Aug 19, 2024

: )
Although the JavaScript engine can handle this situation, attempting to handle both resolve and reject for the same Promise in the code logic indicates that the code structure is not sufficiently rigorous and may have design flaws. This can obscure the intent of the code and potentially confuse other developers.

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Makes me wish we had more testing for ENVIRONMENT_MAY_BE_WEBVIEW.

Are you using this setting (i.e. are you effected by this issue), or did you just see this bug while reading the code?

@sbc100 sbc100 changed the title fix: prevent potential double resolution in XMLHttpRequest handling [webview] prevent potential double resolution in XMLHttpRequest handling Aug 21, 2024
@sbc100 sbc100 merged commit 95e2146 into emscripten-core:main Aug 21, 2024
27 checks passed
@0xlau
Copy link
Contributor Author

0xlau commented Aug 22, 2024

Makes me wish we had more testing for ENVIRONMENT_MAY_BE_WEBVIEW.

Are you using this setting (i.e. are you effected by this issue), or did you just see this bug while reading the code?

Well, Actually I got an error while using k2-fas after packaged in electron environment. And I found that @kripken has already solved this problem. But in order to make the code more rigorous, I submitted this PR.

k2-fsa/sherpa-onnx#1262

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants