- edited description
Embeded ressources may break when editing phpliteadmin.php
Hi Developers of PLA.
Excellent work.
Just I had a problem (I opened an issue too) which is caused by quite strange approach you used in PLA.
I am not a guru of PHP, however, I haven't anywhere seen using embedded resources this way in PLA uses. And obviously (in addition to the fact that this is hard, complex, and non-standard approach), this makes problems to users like me (like in the mentioned issue I experienced).
I've made (in dev version) an update, please review those changes (only in bottom, after output
function), which I am sure, is better, and merge in your project if you can.
thanks in advance.
Comments (12)
-
reporter -
reporter - edited description
-
repo owner Hi. Thanks for your feedback. Please have a look at our repository. You won't find a
phpliteadmin.php
in there. We distribute PLA as a single filephpliteadmin.php
for simple deployment. But we don't develop it like this. In the repository, you find many source files that we use for development. In there, scripts, css and images are separate files. You will also find abuild.php
script and this script produces the single filephpliteamin.php
.We cannot use diffs against
phpliteadmin.php
(especially the totally generated part at the bottom), as this is a generated file. If you want to propose a change, it would need to be based on the files in the repository. If you want to propose changes regarding the embedded ressources at the bottom, this most likely needs to be a change of the generator (build.php
).I will later have a deeper look at what your diff proposes. But I guess it partly breaks the way we develop PLA. Because PLA is not only able to run as single-file. It also runs from many files by running the
index.php
from the repository. At first sight, some of your changes seem to break this behavior. -
reporter Thanks for the response. Actually, do as you wish, just I've faced a real issue, the file doesnt work on my server, and I've pointed to the issue i've opened for that. And the fix I found, i've proposed.
The end of story is that it should work and doesn't matter how you will develop that, but just my 2 cents, that it would be nice if the fix was made, whatever manner you want. (my thougth was what I've suggested, more readable and logical, than complex method it was before). thanks
-
repo owner Sure I will have a look at the problem that you faced and find a solution, maybe also based on your proposal. Thanks for reporting.
-
repo owner I can confirm the original issue
#374. It can happen when the file phpliteadmin.php is edited with a texteditor that changes the newlines within the file. Currently the file has mostly CR+LF to make sure it can be edited with Windows Editor. However, the JavaScript embedded at the end seems to only have LF. Some editors will replace these LF with CR+LF to make the file consistent. When we load the javascript, we do it based on the bytes after the compiler halt position. As the newlines have changed, the javascript is now one byte longer for each line of JS. However, the ressource length is not adjusted. This is why we end up with an incomplete JavaScript (and a broken favicon!). I think the Javascript has only LF newlines because the javascript minifier we use when building removes the CRs.So I know where the problem comes from. Thanks a lot for making us aware. I will think about how to solve it best.
-
reporter thanks for noticing that. ok, whatever you prefer. just that method being used (with indexing and etc...) is quite inconsistent and would be nice if used standard and reliable way, like, like outputing them from function as the example i've given above. many thanks, When you will issue next update, i will update it on my site too.
-
repo owner The standard way of doing what we do (packaging everything in one file) would be to create a PHAR archive. However, most webservers are not configured to handle .phar-files and there are some PHP bugs (related to PHP FPM etc.).
Agreed, your way is more readable and less error-prone than our current approach.
I still need to think about it. Probably I will release a quick fix first that just keeps the CR LF in the JavaScript... Then we can take the time to evaluate different solutions.
-
reporter Ah , you're right about that, just I didn't mean that industrial standard way, which will much burden in this case:) I just meant something simpler alternative and "more accepted" way of doing that, like the suggested change.
thanks for response, I'll not keep you longer in this chat, take care ! nice job!
-
repo owner In version 1.9.8, with commit b1062c89f953a795562a7cf355aa1a056966a639, I took the route to use CR+LF everywhere in order to avoid this problem.
Still, we should think about our resources and how to embed them in a way that is not so fragile. Another way may be to use a CDN and load them from there.
For those users that require PLA to work offline, we could offer a second resources-file that you can just download alongside PLA.
We already load the SQL highlighter from a CDN, so at the moment, it is a bit of a mixture between CDN and embedded resources. Maybe we could embed all core CSS as it is strictly necessary and load all JS from CDN, as PLA still works without the JS.
-
repo owner - changed title to Embeded ressources may break when editing phpliteadmin.php
-
assigned issue to
-
repo owner - changed milestone to 1.9.9
- Log in to comment