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

Image: Don't assume a replacement was specified #822

Closed
wants to merge 1 commit into from
Closed

Image: Don't assume a replacement was specified #822

wants to merge 1 commit into from

Conversation

DannyS712
Copy link
Contributor

Check for event.target.replacement as well.
Follow up #788
Closes #821

Check for `event.target.replacement` as well.
Follow up #788 
Closes #821
@Amorymeltzer
Copy link
Collaborator

Thank you for the contribution. It needed some fixes, so they were made in f1b0573. Your contribution is still included (and still credited to you), with the appropriate modifications. Please feel free to ask about any of the changes.

Amorymeltzer pushed a commit that referenced this pull request Jan 27, 2020
Check for `event.target.replacement` as well.
Follow up #788
Closes #821
Copy link
Collaborator

@Amorymeltzer Amorymeltzer left a comment

Choose a reason for hiding this comment

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

Thanks for the speedy catch here! 🚀 Will make live

@DannyS712
Copy link
Contributor Author

DannyS712 commented Jan 27, 2020

@Amorymeltzer what if replacement exists but not replacement.value? Is it not better to explicitly check?

@DannyS712 DannyS712 deleted the patch-9 branch January 27, 2020 18:30
@Amorymeltzer
Copy link
Collaborator

Yeah, but I couldn't seem to trigger any errors? I was admittedly testing somewhat quickly, are you able to?

@DannyS712
Copy link
Contributor Author

Just using the console:

var event = { target: { replacement: {} } }

event.target.replacement && event.target.replacement.value && event.target.replacement.value.trim()

event.target.replacement && event.target.replacement.value.trim()

First returns undefined, second is Uncaught TypeError: Cannot read property 'trim' of undefined at <anonymous>:1:60

Not sure if this could happen with the form, but just to be on the safe side...

@Amorymeltzer
Copy link
Collaborator

Yeah, couldn't trigger it from the form. I was surprised, tbh

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

Successfully merging this pull request may close these issues.

Uncaught TypeError: Cannot read property 'value' of undefined
2 participants