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

WebApp.Url() doesn't work with media assets #1312

Closed
tidyui opened this issue Aug 20, 2020 · 9 comments
Closed

WebApp.Url() doesn't work with media assets #1312

tidyui opened this issue Aug 20, 2020 · 9 comments

Comments

@tidyui
Copy link
Member

tidyui commented Aug 20, 2020

The new url helpers WebApp.Url(...) and WebApp.AbsoluteUrl(...) doesn't work for media assets as the methods append the site prefix if available. This should be solved by adding new methods WebApp.ContentUrl(...) and WebApp.AbsoluteContentUrl(...) that are suited for uploaded content.

@tidyui tidyui added this to the Version 8.4 SR2 milestone Aug 20, 2020
@tidyui tidyui closed this as completed in bc1e515 Aug 20, 2020
@StephenWRogers
Copy link
Contributor

StephenWRogers commented Feb 12, 2021

I've a problem with the og:image, which I think is only since upgrading to 8.4.1 which might be because of this change. My images are stored in Azure blob storage and the og:image I'm getting now is

https://<-siteurl->https://<-image blob url->

which looks like it's coming from this call to AbsoluteContentUrl as AbsoluteUrlStart will be

https://<-siteurl->

and ContentUrl will be

https://<-image blob url->

which are being appended.

Is this a result of this change, or am I doing something wrong somewhere?

@tidyui
Copy link
Member Author

tidyui commented Feb 12, 2021

I'll try to reproduce this locally and get back to you!

@StephenWRogers
Copy link
Contributor

Did you have any luck with this one?

@gerardvanderkruijs
Copy link

@tidyui The issue described by @StephenWRogers is still present, also in v9.0.0.
Any chance this issue can be re-opened so it can be fixed?

We're using Amazon S3 storage for our images and the og:image tags are rendered as:
http://localhost:5000https://xxx.s3.eu-west-1.amazonaws.com/xxx.jpg

@tidyui
Copy link
Member Author

tidyui commented Mar 19, 2021

@StephenWRogers @gerardvanderkruijs As the initial issue description states WebApp.Url and WebApp.AbsoluteUrl should not be used for media files anymore. Instead WebApp.ContentUrl or WebApp.AbsoluteContentUrl should be used. Are you all using the correct methods or using the old methods in your projects?

@gerardvanderkruijs
Copy link

Hi @tidyui ,
Thanks for the quick reply! I know we shouldn't use it anymore 🙂 But the og tags are rendered from the Piranha.Core module, so it also shouldn't be used there as well 😉
See: https://github.com/PiranhaCMS/piranha.core/blob/master/core/Piranha.AspNetCore/Extensions/PiranhaHtmlExtensions.cs#L82

@tidyui
Copy link
Member Author

tidyui commented Mar 19, 2021

@gerardvanderkruijs Well the line you're referring to actually uses the correct method, i.e AbsoluteContentUrl, so if this produces bad output there has to be some kind of bug :)

@StephenWRogers
Copy link
Contributor

Yeah I'm the same - I'd just commented on this issue as I think it was the changes from this issue that caused my issue (og:image now using AbsoluteContentUrl which produces the above output), but I guess I should probably have raised it as a new issue.

It looks to me like

AbsoluteUrlStart will be

https://<-siteurl->

and ContentUrl will be

https://<-image blob url->

which AbsoluteContentUrl appends together

@gerardvanderkruijs
Copy link

Oh I'm sorry, I think you're right @StephenWRogers, maybe it would have been better if we created a new issue.

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

No branches or pull requests

3 participants