Skip to content

fix: parser error on empty markup string, causing WSOD #179

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

Merged
merged 1 commit into from
Jun 17, 2020
Merged

fix: parser error on empty markup string, causing WSOD #179

merged 1 commit into from
Jun 17, 2020

Conversation

JacobMGEvans
Copy link
Contributor

@JacobMGEvans JacobMGEvans commented Jun 17, 2020

What:
Bugfix related to #161
The Guard was modified now the Error is thrown if rootNode isn't in scope and markup is not a typeof "string"

Why:
The parser was triggering true when all the HTML was removed from the codemirror by the user.

Checklist:

  • Tests
  • Ready to be merged

Screenshot before the fix applied on live site (in original bug report)
Screen Shot 2020-06-15 at 8 50 17 PM

Screenshot locally running with fix applied and the HTML manually removed
Screen Shot 2020-06-16 at 9 24 31 PM

Tests ran after the fix is applied.
Screen Shot 2020-06-16 at 9 29 58 PM

- Error is thrown if rootNode isnt in scope and markup is not a typeof string
@JacobMGEvans
Copy link
Contributor Author

@marcosvega91 I applied your fix, seems to work perfectly 😄

@smeijer
Copy link
Member

smeijer commented Jun 17, 2020

Super 👍, thanks!

@smeijer smeijer changed the title Fix provided allows for the user to remove all HTML without the crash fix: parser error on empty markup string, causing WSOD Jun 17, 2020
@smeijer smeijer merged commit 28e801f into testing-library:develop Jun 17, 2020
@smeijer
Copy link
Member

smeijer commented Jun 17, 2020

@all-contributors please add @JacobMGEvans for code

@allcontributors
Copy link
Contributor

@smeijer

I've put up a pull request to add @JacobMGEvans! 🎉

@JacobMGEvans JacobMGEvans deleted the bugfix/app-crash-issue-161 branch June 17, 2020 13:52
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.

3 participants