Yetangitu / owncloud-apps

Applications for Nextcloud and Owncloud personal cloud server
GNU Affero General Public License v3.0
85 stars 41 forks source link

Reader: Undefined index: title #31

Closed AS-dh closed 7 years ago

AS-dh commented 7 years ago

Reader (ebook reader) 0.8.1 I see several entries with Error PHP Undefined index: title at /www/htdocs/vxxxxxxxx/nextcloud/apps/files_reader/templates/reader.php#16 in the nextcloud admin log. App works fine as far as I can tell...

001101 commented 7 years ago

same issue

why is newest version, which is out for about 14 days not in the stable repo from nextcloud apps??

Yetangitu commented 7 years ago

It should be in the repo but I now see it is not, ie. something must have gone wrong with the submission. Strange as it did say it succeeded. I just submitted the latest version (0.8.3) to the NC and OC repositories, this version contains a fix for the missing $title parameter in templates/reader.php.

BTW, a new version is coming soon which supports more formats, starting with CBR/CBZ ('comics'). Next in the pipeline is a facility to remember reading position, metadata and more such. Eventually I intend to have Reader and OPDS Catalog use the same metadata facility, enabling a self-hosted version of FBReader's 'book network'.

001101 commented 7 years ago

Thanks for the fast fixing, very nais!

Unfortunatly it is not working on my setup, but i just installed it and have a little bit complicated setup (nginx, php5-fpm, cloudflare), so it could be there some issues.

Have also tried in cloudflare dev mode and with and without any caching and other setting tweaks, always the same.

I can see the index left, but there is no text.

My newest Opera browser brings the following:

epub.min.js?v=0.8.3:9 ReferenceError: cleanStartTextContent is not defined
    at EPUBJS.Chapter.cfiFromRange (https://cloud.domain/apps/files_reader/vendor/epubjs/epub.min.js?v=0.8.3:9:4993)
    at EPUBJS.Renderer.mapPage (https://cloud.domain/apps/files_reader/vendor/epubjs/epub.min.js?v=0.8.3:10:20528)
    at EPUBJS.Renderer.updatePages (https://cloud.domain/apps/files_reader/vendor/epubjs/epub.min.js?v=0.8.3:10:14796)
    at EPUBJS.Renderer.afterDisplay (https://cloud.domain/apps/files_reader/vendor/epubjs/epub.min.js?v=0.8.3:10:13538)
    at EPUBJS.Renderer.<anonymous> (https://cloud.domain/apps/files_reader/vendor/epubjs/epub.min.js?v=0.8.3:10:12899)
    at d (https://cloud.domain/apps/files_reader/vendor/epubjs/epub.min.js?v=0.8.3:9:23216)
    at EPUBJS.Hooks.register.highlight (https://cloud.domain/apps/files_reader/vendor/epubjs/hooks/extensions/highlight.js?v=0.8.3:10:18)
    at https://cloud.domain/apps/files_reader/vendor/epubjs/epub.min.js?v=0.8.3:9:23338
    at Array.forEach (native)
    at EPUBJS.Renderer.a.triggerHooks (https://cloud.domain/apps/files_reader/vendor/epubjs/epub.min.js?v=0.8.3:9:23318)
(anonymous) @ epub.min.js?v=0.8.3:9
trigger @ epub.min.js?v=0.8.3:8
(anonymous) @ epub.min.js?v=0.8.3:8
setTimeout (async)
va.after @ epub.min.js?v=0.8.3:8
_onError @ epub.min.js?v=0.8.3:8
t @ epub.min.js?v=0.8.3:8
pa @ epub.min.js?v=0.8.3:8
characterData (async)
(anonymous) @ epub.min.js?v=0.8.3:8
ja @ epub.min.js?v=0.8.3:8
u @ epub.min.js?v=0.8.3:8
r @ epub.min.js?v=0.8.3:8
s @ epub.min.js?v=0.8.3:8
B.c @ epub.min.js?v=0.8.3:8
iframe.onload @ epub.min.js?v=0.8.3:10
Yetangitu commented 7 years ago

That looks quite similar to this bug report for the futurepress epub renderer (epub.js) used in Reader:

https://github.com/futurepress/epub.js/issues/483

While epub.js does give good results with most epub documents it is rather picky when it comes to handling less-than-perfect files. It could just be that it does not like your epub? If you send me the file I can see if it works here. Try with another epub first to check whether that might be the issue.

If this does not work you could try to replace the minified version of epub.js (located in vendor/epubjs/epub.min.js) with the non-minified version found here:

https://raw.githubusercontent.com/futurepress/epub.js/master/build/epub.js

Download this file, move the existing minified version away and copy the non-minified version to vendor/epubjs/epub.min.js so that Reader can find it. Reload the page from your server and try again. If it still fails make sure you actually got the non-minified version for epub.min.js and not a stale cached version.

If this fixes the problem it is most likely caused by some side effect of minification, even though that should not happen...

Yetangitu commented 7 years ago

Checking that epub.js bug report I noticed there is a fix for this issue. I'll include it in the next release, probably tomorrow.

https://github.com/futurepress/epub.js/pull/535

001101 commented 7 years ago

That works!!

Thanks very much for your fast solution, very nais. I always prefer unminified Javascripts, because of such issues, they can get minified by Proxys, if needed.

But the nightmode need some tweaks:

2017-02-02_213440

Yetangitu commented 7 years ago

Try with another epub, preferably from a different publisher. Does this appear the same in other browsers, especially non-Blink-based such?

Nightmode is somewhat tricky as it currently only sets CSS parameters for the container. While this generally works OK the results can be messed up by any CSS in the actual epub, especially when that CSS contains !important clauses.

Yetangitu commented 7 years ago

The problem reported in https://github.com/Yetangitu/owncloud-apps/issues/31#issuecomment-276824916 is fixed (or should be, please test...) in 0.8.4

001101 commented 7 years ago

Thank you very much, it is working now flawlessly in 0.84! Nightmode also working good in other file, but the nightmode text color settings get not saved, is it normal? Please use a bit brighter color for better contrast as default.

Yetangitu commented 7 years ago

Settings will be saved in one of the upcoming versions, probably 0.9, 'one of these days now'...

As to default colour choices, this is a very personal thing - and one of the reasons why it is important to be able to save those choices. I use less contrasting defaults because those are easier on the eyes when reading on light-emitting screens (eg. LCD/OLED).

AS-dh commented 7 years ago

Thanks for this useful app. I'm looking forward to the future development. From what Yetangitu lists above, if I may take the liberty to express my very personal preferences:

  1. facility to remember reading position (and I suppose: sync it across devices?)
  2. CBR/CBZ ('comics')
Yetangitu commented 7 years ago
  1. position and preferences are nearly ready. This will be remembered per book, settings will work on any device where Reader works. 'syncing'? That sounds more like something for dedicated reader devices or reader apps (eg. FBReader), few of which support syncing. FBReader happens to be a program which does support this, but thus far only to the 'Book network' (the address of which is hard-coded in the program) using its own protocol. I might make a syncing adapter for a special build of FBReader (which I'd also have to make) which support self-hosted 'book network' functionality.

  2. This is ready and will be released RSN, the wait is for step 1. to be finished.

BTW, my personal preferences are for a) working search functionality, b) better metadata support in NC/OC and c) a 'bookshelf', comparable to the way Gallery presents images. b) will be partly implemented in one of the coming releases by using the metadata support in OPDS Catalog, a) depends on getting working - and I mean really working, capable of indexing a library of 400.000 publications - search in NC/OC and c) would be something to either add to Gallery as a plugin or to Reader. I'd prefer for this to be as unobtrusive as possible, eg. by using the existing Gallery switcher icon (top-right) to switch between directory and shelf view.

001101 commented 7 years ago

If you would like to implement a position saving feature, then look at the Moon+ Reader Pro, he is my personal choice for reading and has this feature included, so you can adopt the file format and structure, which is a faked zip with a sqllite inside:

About Moon+ Reader's Backup Files Moon+ Reader's backup files have the file extension .mrpro (on the Pro version anyway). This is actually a Zip file. If you extract it, inside is a folder called "com.flyersoft.moonreaderp". Inside that is a bunch of files. In my case, there is typically about 60 files, most of them numbered files from 1.tag to 57.tag files. The important one you want is one of the high numbers. Look at the file size also to help you. One of them is actually the SQLite3 database. Mine was about 1.1 MB, while the rest were tiny, which tipped me off it was the real contents.

Source: https://awesomelinux.blogspot.de/2014/06/how-to-move-files-for-moon-reader-while.html

Also interesting about the file: Moon+ Reader uses SQLite version 3 underneath and all the highlighting, bookmarks, notes, etc. are stored in these.

Yetangitu commented 7 years ago

I already have a position saving feature which works across different file formats (epub and CBR/CBZ, currently). For epub I use CFI, for CBR/CBZ page (or image) numbers. As to syncing position between devices (other than browsers running NC/OC with Reader), that can only be done when the device supports syncing to a server. I don't think Moon+ Reader supports this, nor does Coolreader or Aldiko. For some (eg. Aldiko) there are external apps which can sync between different devices but those only work for that specific app and a specific 'cloud server' (eg. Aldiko sync with Dropbox). Google Play Books can sync between devices and browsers but, again, only to users of Google Play Books. FBReader does it through the FBReader Book Network. Missing in this equation is a one-for-all service but implementing one is a lot of work and would lead to hackish and brittle solutions for the closed-source products. Implementing one for the free software readers - Coolreader and FBReader - is feasible.

Yetangitu commented 7 years ago

BTW, nearly all Android apps use SQLite for configuration purposes, just have a look in the database folders (/data/data/app.name/databases) for some examples. This goes for Android itself as well, e.g.

sqlite3  /data/data/com.android.providers.settings/databases/settings.db .dump

(Android 4.4.x, newer versions may use different locations, needs root)

001101 commented 7 years ago

I am more for intercompatibility than another homebrew