Support for pwg format served alternative format files #27
Conversation
Works with Piwigo hosted at a folder on a website Example nginx configuration used with these changes
|
|
||
| if (in_array($type, $arrayZips) && in_array($original_extension, $arrayExtensions)) | ||
| { | ||
| return $original_extension; |
There was a problem hiding this comment.
Why do you return the extension here and not the mime type ?
There was a problem hiding this comment.
This code is heavily based on code I found on the internet. Using the mime type of the file is inappropriate since it is a zip file. So, you want to return a mime type that identifies it as an ms word document or excel document etc. I'm not sure if anyone actually serves files of this type, but it didn't seem to harm by leaving it in.
There was a problem hiding this comment.
From what I recall, the place I got this from said there is no mime type specific to e.g. an ms excel or ms word file, hence just use the file extension as that. Whether that is true or not I did not check.
There was a problem hiding this comment.
I don't think it makes sense for a method that suppose to return the mime type to return the extension instead.
I'm not sure what the issue with zips is, but I doubt if it's relevant to what this plugin does.
If you can find the source of this code and the reason for it we can reconsider.
| return $original_extension; | ||
| } | ||
| // strip the charset encoding part | ||
| $type = preg_replace('#^([a-zA-Z*]*/[a-zA-Z*]*).*$#','$1',$type); |
There was a problem hiding this comment.
Won't it be better to use FILEINFO_MIME_TYPE so you won't have the encoding in the first place?
Also won't having the encoding cause the previous checks to always be false as the mime arrays do not contain any encoding info ?
There was a problem hiding this comment.
FILEINFO_MIME_TYPE isn't always available depending on php version. I'm not well versed in php. I've taken such statements at face value.
There was a problem hiding this comment.
From PHP doc FILEINFO_MIME_TYPE is available since php 5.3.0 so we can use it without any worries.
It seem like this makes this whole function obsolete if we don't need to hack around zip files
| // the the base path for the original file | ||
| $base_img_path = preg_replace('#(.*/)[^/]*\.[^.]*$#','$1',$original_file_path); | ||
| // get the filename without extension of the original file | ||
| $base_img_name = preg_replace('#.*/([^/]*)\.[^.]*$#','$1',$original_file_path); |
There was a problem hiding this comment.
All this regex looks a bit comple. Any reason not to use pathinfo here instead ?
There was a problem hiding this comment.
not specifically. I know regexp, I don't know php :-)
There was a problem hiding this comment.
So it's probably better to see if pathinfo info gives the desired results and use it instead
|
I don't mind what changes you make to achieve what this code does, but in a simpler more php minded way. |
|
@extorn I'm not sure from your other comments if you want to work on this PR or you just want me to try and add this logic in (will probably take longer). |
|
@yonjah, I don't really want to work on the PR thanks. I read through the code, gained a basic understanding sufficient to provide some kind of a fix. PHP isn't a language I am confident in, so I am certain that you'll achieve a working version quicker on your own from the boost this code has hopefully provided than I would by searching the net for PHP example code I can cobble together. It stands to reason that you'll understand this code base substantially better than I. |
|
I'll fork and try this thing now. |
|
Sorry, this does not work for me. I've got .pdf files in my gallery and added ".pdf" / "application/pdf" to the two arrays changed in code above but then my gallery was broken. No more pictures are viewed and no pdf files miss the /sessId/ prefix leading to a http 404 error code. |
Have you tried with the base plugin ? I'm not sure you need this changes. Also 404 error might be an issue with your http server config. |
|
I'll gather this info and report back. The v1.0.1 base does not work, too. I've used "charlies-content" plugin to displays the pdf files directly in the gallery. if I disable the nginx stuff and piwigo_privacy, it works correctly. |
Maybe the plugin is using another path that is not defined in the basic nginx conf example ? |
This adds support for resources of any type served under the pwg_format child folder of a piwigo site.