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

Ensure the svg file loads properly before we access it's properties #186

Merged
merged 1 commit into from Mar 26, 2024

Conversation

dkotter
Copy link
Collaborator

@dkotter dkotter commented Mar 25, 2024

Description of the Change

In #154 some refactoring was done to help with performance of calculating the svg dimensions. But this inadvertently removed some code that was in place to ensure an svg could load properly before trying to access its dimensions.

We have a call to simplexml_load_file and then we access the svg attributes from that. But if that function returns false, this fails. Previously we had an if statement in place to prevent this from being a problem. This PR adds that same thing back but instead of a large nested statement, we return false early if the svg can't be loaded.

Closes #184

How to test the Change

You need to have a valid svg file that you upload and then delete this from your disk (but not from WordPress). In addition, you need to delete the meta for that attachment so we don't have the height and width saved (otherwise these will be returned and the simplexml_load_file call will never happen).

Once you ensure both of those are done, you should see a fatal error prior to this PR. With this PR checked out, you shouldn't get a fatal anymore.

Changelog Entry

Fixed - Ensure the svg file can be loaded before we try accessing it's attributes.

Credits

Props @dkotter, @metashield-ie, @ocean90

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@dkotter dkotter added this to the 2.3.0 milestone Mar 25, 2024
@dkotter dkotter self-assigned this Mar 25, 2024
@dkotter dkotter requested a review from jeffpaul as a code owner March 25, 2024 20:18
@dkotter dkotter requested review from a team and faisal-alvi and removed request for jeffpaul and a team March 25, 2024 20:18
Copy link
Member

@faisal-alvi faisal-alvi left a comment

Choose a reason for hiding this comment

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

@dkotter thanks for the fix. I tested but the issue did not regenerate for me. However, the code LGTM.

image image image

@dkotter dkotter merged commit 941da6c into develop Mar 26, 2024
13 checks passed
@dkotter dkotter deleted the fix/184 branch March 26, 2024 13:57
@dkotter dkotter modified the milestones: 2.3.0, 2.2.4 Mar 26, 2024
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.

Fatal Error in Version 2.2.3, Calling member function attributes() on a Bool
3 participants