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

Fix crash on undefined image url #967

Merged
merged 2 commits into from
May 12, 2019
Merged

Fix crash on undefined image url #967

merged 2 commits into from
May 12, 2019

Conversation

mkevins
Copy link
Contributor

@mkevins mkevins commented May 9, 2019

Description

This PR updates the Gutenberg reference to bring in the changes in this companion PR: WordPress/gutenberg#15532.

This PR adds a check to ensure attributes.url exists before attempting to call indexof. Currently, there are scenarios which can result in attributes.id being truthy, while attributes.url is null or undefined for an image block.

When an image is cancelled before it is finished uploading, the url is set to null, but the id of the block remains. The next time the post is edited, the local draft will contain the state which triggers the crash.

Testing steps:

This was tested using the steps here: wordpress-mobile/WordPress-Android#9768 (comment)

Types of changes

This is a bug fix for the crash discovered here: wordpress-mobile/WordPress-Android#9768

@peril-wordpress-mobile
Copy link

Warnings
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

@mkevins mkevins requested review from marecar3 and removed request for hypest May 9, 2019 14:26
@hypest
Copy link
Contributor

hypest commented May 10, 2019

Hey @mkevins , I guess the v1.4.0 part of the branch name should be bumped to 1.4.1 since 1.4.0 is already deployed. One way to address this without creating a new PR is to make this PR target a hotfix/1.4.1 branch instead of master. If and when we decide to deploy that as a "full" hotfix, we can open a PR from hotfix/1.4.1 to master to track it. How does that sound?

@mkevins mkevins changed the base branch from master to hotfix/v1.4.1 May 12, 2019 11:13
@mkevins mkevins merged commit 0f01bb3 into hotfix/v1.4.1 May 12, 2019
@mkevins mkevins deleted the hotfix/v1.4.0 branch May 12, 2019 11:13
@mkevins mkevins removed the request for review from marecar3 May 12, 2019 11:23
@mkevins
Copy link
Contributor Author

mkevins commented May 12, 2019

I've created the hotfix branch with the correct version, and now merged this branch to that one. I have opened a PR on that branch targeting master.

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.

2 participants