seblucas / cops

Calibre OPDS (and HTML) PHP Server : web-based light alternative to Calibre content server / Calibre2OPDS to serve ebooks (epub, mobi, pdf, ...)
http://blog.slucas.fr/en/oss/calibre-opds-php-server
GNU General Public License v2.0
1.43k stars 229 forks source link

new open button (1.1.2) is erroneous #402

Closed woidi closed 5 years ago

woidi commented 6 years ago

I'm not using url rewriting and the ebook files are delivered directly (without using fetch.php). The button creates a href of "pathname/filename.pdf&view=1" which results in "not found" . imho the ampersand should be a question mark ?

seblucas commented 6 years ago

I guess you meant you're using URL rewriting ? right ?

If that's true then I can confirm the bug and will fix it this weekend.

If not then please explain the problem a little more, thanks in advance.

woidi commented 6 years ago

thank you for having a look. I did not dig into the code very deep and do not yet understand, where and how data.url for the template files gets filled. Does it help if you see https://git:hub@chorfreu.de/vcl/cops/index.php?page=13&id=8 yourself? You can check https://git:hub@chorfreu.de/vcl/cops/config_local.txt as well.

seblucas commented 5 years ago

Could you test with latest master and confirm your problem is fixed.

woidi commented 5 years ago

Sorry Sébastien, I can not confirm the fix. But again I'm NOT using URL rewriting.

Inside a book list like https://git:hub@chorfreu.de/vcl/cops/index.php?page=18&id=5 the open button referenced the PDF itself without view parameter. But inside a bookpopup like https://git:hub@chorfreu.de/vcl/cops/index.php?page=13&id=8 the open button referenced https://chorfreu.de/vcl/cops/undefined which gave an error. This is because you forgot to add "viewUrl" => $data->getViewHtmlLink () in getFullBookContentArray in JSON_renderer.php line 86.

And my installation is NOT using an absolute path, thus the last line of code in function getLink() in data.php is used, which did not respect the parameter $view. I hacked .($view?"?view=1":"") into my installation: return new Link (str_replace('%2F','/',rawurlencode ($book->path."/".$filename)).($view?"?view=1":""), $mime, $rel, $title); but don't know about possible side effects.

marioscube commented 5 years ago

Hi Sébastien. I still like COPS a lot, but since the commits on february 25 2018 COPS does not work consistently.

There are problems with:

Have you tested with your kobo? (I did not, but it must be gruesome to do...).

I'm still doing some testing, but i think i'm reverting back to a version of cops before february 2018.

Despite what I have written above, I still thank you for COPS.

Ideally I would like COPS to revert back to the status it was in before february 25 218, but as I'm only a happy user that up to you of course.

seblucas commented 5 years ago

@woidi

Inside a book list like https://git:hub@chorfreu.de/vcl/cops/index.php?page=18&id=5 the open button referenced the PDF itself without view parameter.

That's normal with relative path. If you use a relative path, your epub files are served directly by your webserver, I cannot change anything in the headers so there will be no difference between the download and open button.

But inside a bookpopup like https://git:hub@chorfreu.de/vcl/cops/index.php?page=13&id=8 the open button referenced https://chorfreu.de/vcl/cops/undefined which gave an error. This is because you forgot to add "viewUrl" => $data->getViewHtmlLink () in getFullBookContentArray in JSON_renderer.php line 86.

Indeed I missed some modifications, I'll change that soon (before the end of week).

And my installation is NOT using an absolute path, thus the last line of code in function getLink() in data.php is used, which did not respect the parameter $view. I hacked .($view?"?view=1":"") into my installation:

I don't think that's needed, see my answer for your first point.

seblucas commented 5 years ago

@marioscube

Hi Sébastien. I still like COPS a lot, but since the commits on february 25 2018 COPS does not work consistently.

I'm sorry about that, I intend to fix that.

absolute vs relative paths (and symlinked paths?)

absolute vs relative paths should be fixed soon. I remember your problem with symlinked paths, As I said before, I never tried that before, I'll test that during the weekend. Do you use relative or absolute path with your symlink (I assume relative but I prefer to be sure).

URL rewriting (for kobo and Marvin3) vs without URL rewriting.

You use OPDS with Marvin ? I don't think I changed anything in the OPDS for quite some time. Can you be a little more specific or point me to the issue.

view .... but with what add-on? (macOS and Linux on Firefox)

I use it with Edge on windows which has a embedded Epub reader. And I like it better than current Epub reader. On my android devices I use Chrome and the download button which open BookReader. And on my Kobo, again the download button.

Never checked if there is an epub reader in Firefox.

OS-es and versions of browsers with or without add-ins. (version 1.1.2 works more or less on my Synology (older apache?), while the same version give more errors on my Debian Jessie / Stretch servers with apache).

I updated some dependencies in that version, can you be more specific with the errors ?

Have you tested with your kobo? (I did not, but it must be gruesome to do...).

Yes, that the only way I add any books on my Kobo, I never connect it with USB and always use the epub version of all books I buy from Kobo.

Ideally I would like COPS to revert back to the status it was in before february 25 218, but as I'm only a happy user that up to you of course.

I agree I should not have merge the pull request adding the view buttons. I did not have tested it properly, it wasn't always working, I spent a weekend fixing some bugs but did not find all of them.

I think I have mostly fixed URL Rewriting, Relative path should also be fixed. I'll tackle the rest one bug/regression at a time.

marioscube commented 5 years ago

@seblucas

I had so many strange errors and inconsistencies that I decided to reinstall Apache/PHP/COPS.

The main problem I had (have) is that the way I had everything installed did NOT make .htaccess work. After if fixed this I had only one (root/major) problem left, and that's the one @woidi started this thread with. And you are going to fix that ;-)

Below some answers if relevant after my reinstallation:

@marioscube

Hi Sébastien. I still like COPS a lot, but since the commits on february 25 2018 COPS does not work consistently.

I'm sorry about that, I intend to fix that.

Thank you!

absolute vs relative paths (and symlinked paths?)

absolute vs relative paths should be fixed soon. I remember your problem with symlinked paths, As I said before, I never tried that before, I'll test that during the weekend. Do you use relative or absolute path with your symlink (I assume relative but I prefer to be sure).

I use a relative path to a symlink (./symlinkpath). But it seems to work now.

URL rewriting (for kobo and Marvin3) vs without URL rewriting.

You use OPDS with Marvin ? I don't think I changed anything in the OPDS for quite some time. Can you be a little more specific or point me to the issue.

I can use OPDS with Marvin classic and Marvin 3. When using the internal web browser for Marvin classic I do NOT need URL_rewriting, for Marvin 3 (just like kobo) I need URL_rewriting.

But it seems to work now.

view .... but with what add-on? (macOS and Linux on Firefox)

I use it with Edge on windows which has a embedded Epub reader. And I like it better than current Epub reader. On my android devices I use Chrome and the download button which open BookReader. And on my Kobo, again the download button.

Never checked if there is an epub reader in Firefox.

I use (mainly) macOS with Firefox. iOS (iPad) with Marvin classic and Marvin 3 with either OPDS or the build-in browser. Nice for testing cops with alternative hard- and software. Firefox used to have a working epub-reader (epubreader) as an add-in, but the latest versions of firefox make click and view for add-ins not possible (afaik).

So You see I have not much use for separate download an view buttons in the browser interface of cops.

OS-es and versions of browsers with or without add-ins. (version 1.1.2 works more or less on my Synology (older apache?), while the same version give more errors on my Debian Jessie / Stretch servers with apache).

I updated some dependencies in that version, can you be more specific with the errors ?

Probably related to .htaccess not working properly.

Have you tested with your kobo? (I did not, but it must be gruesome to do...).

Yes, that the only way I add any books on my Kobo, I never connect it with USB and always use the epub version of all books I buy from Kobo.

Ideally I would like COPS to revert back to the status it was in before february 25 218, but as I'm only a happy user that up to you of course.

I agree I should not have merge the pull request adding the view buttons. I did not have tested it properly, it wasn't always working, I spent a weekend fixing some bugs but did not find all of them.

I think I have mostly fixed URL Rewriting, Relative path should also be fixed. I'll tackle the rest one bug/regression at a time.

I'll wait for it.

Tank you for cops (and fixing it).

horus68 commented 5 years ago

This should be closed now? @seblucas