search

uSked / mosaico-php-backend

A PHP backend for Mosaico
GNU General Public License v3.0
32 stars 34 forks source link

handling of image urls #8

Closed bwl21 closed 8 years ago

bwl21 commented 8 years ago

in commit d1e5da5 you changed editor.html not to use absolute URLs. This has the drawback that mosaico does not yield image references which match the regular expressions of dl/image.php. Therefore no static images are created.

see https://github.com/voidlabs/mosaico/issues/57

what was the reason that you did this change? Otherwise we should revert this.

bago commented 8 years ago

I changed mosaico, too. The "node.js" backend was returning relative urls, while jquery-file-upload "reference" (php) implementation always returns absolute urls. So I changed the node.js backend to return absolute urls and this works fine if you use absolute urls in Mosaico.init.

We could even change the "convertedUrl" method could always invoke the backend without trying to understand if the backend will be able or not to deal with that image.

BTW, given at the end we'll need absolute urls for images, we probably should always work with absolute urls (in my deployment I have to use relative urls, so that's why I'm still evaluating this, to understand if my deployment is a "special case" or not).

bwl21 commented 8 years ago

I actually started to change the php backend to yield absolute urls. It is easy if editor and backend are on the same server. It is more difficult if backend is on a different server. Maybe I have to add a configuration parameter for the backend to specify the base url for images.

bago commented 8 years ago

What about using $_SERVER['HTTP_HOST'] so that you reply with the same hostname you've been invoked?

bwl21 commented 8 years ago

I think of this. I need to take into account that the backend might be installed in a subdirectory. But this can be solved.

preg_replace('/upload\/$/', '', $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'])
mherbold commented 8 years ago

Hello - apparently something in Mosaico has changed that breaks dl/index.php - I will have to get latest Mosaico and see what needs to be fixed. I will be able to get back on this project in a couple of weeks.

bwl21 commented 8 years ago

It now works on my server with the help of Stefano. It is indeed a bit fragile. It required that the backend delivers the same URL as the referring paged was served. This is now the case with my PR. But the Regular Expressions in the backend had problems too (unquoted slashes)

mherbold commented 8 years ago

Ah I see - yes my code does assume that the images live on the same server as the referring page. I can change it so that you can specify in a config setting the root URL of the images. I will also look into the regex issue. Can you clarify what you mean by unquoted slashes?

bwl21 commented 8 years ago

I have fixed the regular expressions already in the PR. There things like '/upload/$/ which should have been '/upload\/$/'.

I change config.php such that it can have a $baseurl. And then I set this to be computed from $_SERVER. But it is the preparation to hold the images somewhere else.

It is also a matter of workflow of the user if the images are uploaded via mosaico, or provided e.g. by ftp.

mherbold commented 8 years ago

I refactored stuff and you should now be able to use completely separate settings for the file system paths and URL paths. I looked for /upload/$/ but couldn't find any in my code.

bwl21 commented 8 years ago

Did you have a look in the Pull requests? If you start refactoring before the merge they probably will not merge anymore.

mherbold commented 8 years ago

I looked at the pull requests and moved over the relevant changes manually. Let me know if the new paths stuff doesn't work for you. If there's something critical to make this work for you that I missed let me know that as well.

bwl21 commented 8 years ago

continue the disucssion in #11