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

Fixed _Beans_Image_Editor::run() can return path instead of url #353

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

Tsquare17
Copy link
Member

If the image doesn't exist, and image editing fails, the absolute path is returned instead of the src.

@christophherr
Copy link
Member

Hey @Tsquare17,
What does this change fix?
What is the benefit of this change?
What could break from this change?

@Tsquare17
Copy link
Member Author

Tsquare17 commented Dec 8, 2018

@christophherr,
So, I came across this when doing some testing locally. I created a post and uploaded an image. But, when viewing the site, the images were broken, having an src that contained the absolute path to the original image.

I found the problem was caused actually caused by a missing php module on my system, but what happens is that if an error occurs attempting to edit the image, instead of returning:

return $this->get_image_info( beans_path_to_url( $this->rebuilt_path ), true )

with the src to the edited image, beans returns:

$this->get_image_info( $this->src )

which doesn't use beans_path_to_url, resulting in an image with src containing the path.

for example:
/var/www/html/testing/wp-content/uploads/2018/11/image.jpg
instead of:
http://testing.com/wp-content/uploads/2018/12/image.jpg

@christophherr
Copy link
Member

Thank you for providing more details 👍
Could you check if we changed this in the 1.5 update or if it has been like this in 1.4?
Could it make sense to check if the PHP extension/module is available and throw an exception instead?

@Tsquare17
Copy link
Member Author

It looks like it was introduced when refactoring after 1.5. Here is the commit.

Do you mean to throw an exception rather than returning the original src?

@christophherr
Copy link
Member

christophherr commented Feb 12, 2019

Thanks for the detective work, @Tsquare17 👍

Do you mean to throw an exception rather than returning the original src?

You said that you were missing a PHP module and with the module things were working.
I was wondering what is better; to run the extra function or let people know that a common/needed php extension/module is missing.
Was it a common module that was missing or something out of the ordinary?

@Tsquare17
Copy link
Member Author

I believe it was php-gd that was missing. People aren't likely to run into that module missing unless they're setting up their own servers and neglect to install it, like someone here...

Failing gracefully would be the easy fix, and what I think was initially intended, but if you think a notice would be more appropriate, then we can certainly go that route. I'm not sure what the best way to go about that would be, but if you could help me in the right direction, I can give it a shot.

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.

None yet

2 participants