xy2z / PineDocs

A fast and lightweight site for viewing files
GNU General Public License v3.0
141 stars 19 forks source link

Add support of cyrillic filenames #114

Closed vvrein closed 4 years ago

vvrein commented 4 years ago

Here is my try to add cyrillic filenames support to PineDocs. I replaced basename and pathinfo with preg_split, since they does not works properly with real utf-8 which appears as loosed basename. And utf8_decode was replaced to escape пробела.md symbols, since as I found in template, that we tell browser, that encoding is utf8. But php docs says that This function converts the string data from the UTF-8 encoding to ISO-8859-1.

Original: image Fixed: image

Also, I'm not php coder, so sorry in advance. Cheers!

xy2z commented 4 years ago

Thanks!

I can confirm it works on title, navigation and rendering.

But, when I refresh the page while on a page with cyrillic filename it doesn't work, eg:

Error: could not load file: %D1%84%D0%B0%D0%B9%D0%BB.txt
File not found.

Maybe we should base64 encode isntead of utf8 encode in the AJAX request, but im not sure where the bug is. It seems PHP cant find the filename from the ajax request.

vvrein commented 4 years ago

Yup, today I also faced this thing, and will try to solve in case I have enough free time

vvrein commented 4 years ago

Random thought - somewhere in the code there is lacks of urldecode, so when page is refreshed - php tries to open encoded url as is. While ascii urls does not encoded, so they opens.

vvrein commented 4 years ago

Yup, here is ascii filename with space in it

[testing@shelter2 content]$ echo "space check" > "space check.md"
[testing@shelter2 content]$ git branch
  cyrillic-names
  deploy
  develop
* master
[testing@shelter2 content]$

image

vvrein commented 4 years ago

Quick fix, but seems working :) Since I don't understand initial aim of utf8_decode - this changes might broke something... But in my setup (nginx + php-fpm 7.4.11) everything works as intended:

xy2z commented 4 years ago

Ah thats a bug! Maybe we can urlencode the paths in JS and the urldecode in PHP? Or did you fix it?

IIRC i used utf8_decode so characters like "æ, ø, å" would work, but im not sure. But this seems to be better 👍

vvrein commented 4 years ago

Weird... I didn't push anything, wait a bit :)

vvrein commented 4 years ago

image Seems they displayed ok right how

xy2z commented 4 years ago

Nice! Only little thing now missing is the autofocus in the navigation on page reload, and opens the dir-tree. This currently works on non cyrillic files.

You can fix it in the .JS by added decodeURIComponent() around window.location.hash in public/js/PineDocs.js

var file = $('a.link_file[href="' + decodeURIComponent(window.location.hash) + '"]')

...

$('a.link_file[href="' + decodeURIComponent(window.location.hash) + '"]').click()

Here: https://github.com/xy2z/PineDocs/blob/master/public/js/PineDocs.js#L54-L57

vvrein commented 4 years ago

I've replaced window.location.hash with decodeURIComponent(window.location.hash) everywhere when it used like a key or string, and when we possibly wants it to be decoded.

Does this would be ok, or we need changes only in L54-L57?

diff --git a/public/js/PineDocs.js b/public/js/PineDocs.js
index f4bbc2d..fdba530 100644
--- a/public/js/PineDocs.js
+++ b/public/js/PineDocs.js
@@ -49,15 +49,15 @@ $(function() {
                self.render_errors()

                // Autoload file from URL anchor tag.
-               if (window.location.hash.length >= 2) {
+               if (decodeURIComponent(window.location.hash).length >= 2) {
                        // Check if file exists.
-                       var file = $('a.link_file[href="' + window.location.hash + '"]')
+                       var file = $('a.link_file[href="' + decodeURIComponent(window.location.hash) + '"]')
                        if (file.length) {
                                // File exists
-                               $('a.link_file[href="' + window.location.hash + '"]').click()
+                               $('a.link_file[href="' + decodeURIComponent(window.location.hash) + '"]').click()
                        } else {
                                // File does not exist.
-                               self.render_hidden_file(window.location.hash.substr(1));
+                               self.render_hidden_file(decodeURIComponent(window.location.hash).substr(1));
                        }
                } else {
                        // Load index file.
@@ -151,8 +151,8 @@ $(function() {
                $('title').text(config.title + ' | ' + data.basename)

                // Scroll to last position.
-               if (self.scroll_top[window.location.hash]) {
-                       self.elements.file_content.scrollTop(self.scroll_top[window.location.hash])
+               if (self.scroll_top[decodeURIComponent(window.location.hash)]) {
+                       self.elements.file_content.scrollTop(self.scroll_top[decodeURIComponent(window.location.hash)])
                } else {
                        self.elements.file_content.scrollTop(0)
                }
@@ -193,7 +193,7 @@ $(function() {
                        link.addClass('active')

                        // Remember scroll position.
-                       self.scroll_top[window.location.hash] = self.elements.file_content.scrollTop()
+                       self.scroll_top[decodeURIComponent(window.location.hash)] = self.elements.file_content.scrollTop()

                        // Already loaded before?
                        if (self.loaded[href]) {

Or, can we make something like this

        var PineDocs = function() {
                var self = this
...

                self.render_errors()
                self.urihashpart = decodeURIComponent(window.location.hash)

And then use self.urihashpart everywhere, where we need decoded part?

UPD

Hmm, seems no. At least self.urihashpart = decodeURIComponent(window.location.hash) needs to be redefined in each function it's called. So back to decodeURIComponent(window.location.hash) in each place it used :)

xy2z commented 4 years ago

In my testing you only have to do it on line L54-L57

Scroll position still works, and cyrillic filenames in a hidden dir also still works.

And also 1 line here: https://github.com/xy2z/PineDocs/blob/master/public/js/PineDocs.js#L474 You can trigger this by loading a url that doesnt exist. This should be changed to:

self.render_error_message('Error: could not load file: <u>' + decodeURIComponent(path) + '</u><br />File not found.')
vvrein commented 4 years ago

Thanks! I've updated those lines.

xy2z commented 4 years ago

Tested and works! Thanks @vvrein I'll make a new release soon