mojolicious / mojo

:sparkles: Mojolicious - Perl real-time web framework
https://mojolicious.org
Artistic License 2.0
2.66k stars 576 forks source link

The image helper now produces absolute paths #2107

Closed jixam closed 4 months ago

jixam commented 9 months ago

Steps to reproduce the behavior

perl -E 'use Mojolicious; say join ": ", $Mojolicious::VERSION, Mojolicious->new->build_controller->image("test.png")'

Expected behavior

We noticed this change in behavior which does not seem to be mentioned in Changes.

Before we adjust our application we wanted to check that the change is intended?

Actual behavior

Previous output:

9.33: <img src="test.png">

New output:

9.34: <img src="/test.png">
kraih commented 9 months ago

Yes, this is intentional. The generated URL is supposed to be relative to the static directory of the application.

jixam commented 9 months ago

All right, thank you for the clarification.

This is quite a breaking change for us as we have been treating the parameter as just the verbatim src attribute, doing things like image("https://example.org/test.png"). But we will cope :)

kraih commented 9 months ago

Absolute URLs are actually supposed to be passed through unchanged. If that doesn't work currently it would be incompatible with mojo.js and therefore a bug.

rawleyfowler commented 9 months ago

We're effected by this as well, I'll take a crack at a PR.

rawleyfowler commented 9 months ago

Here's an instance of the behavior in one of our projects:

src="/https://reasonml.github.io/img/reason.svg"
sergiotarxz commented 4 months ago

I am getting to work in this. PD1: Should I change this behavior in sub _file_url? PD2: I have done a pull request testing the behavior people said was broken. #2149 PD3: I am under the impression that what I tested only duplicates an existing test from #2112 which tests the inner subroutine instead of the helper image. Maybe this issue should be closed?

jixam commented 4 months ago

I will go ahead and close this issue since it was mostly fixed by #2112.