koreader / kindlepdfviewer

(DEPRECATED, please use KOReader instead) A PDF (plus DJVU, ePub, TXT, CHM, FB2, HTML...) viewer made for e-ink framebuffer devices, using muPDF, djvulibre, crengine
GNU General Public License v3.0
498 stars 98 forks source link

bookmark jumping in epub messed up in certain case #856

Open dracodoc opened 9 years ago

dracodoc commented 9 years ago

There is a bookmark jumping feature added by my request, which use alt+k/l to jump between bookmarked pages. It has been working great in pdf for a long time for me, but I just found it have a problem in epub reading.

Suppose you have several bookmarks and you are using alt+L to jump from bookmark 1 to 2, then use page down to flip 1 page normally, then use alt+L to jump to bookmark 3, instead it go to bookmark 1. If you tried alt+K after page down, it didn't go to the previous bookmark, instead nothing happens.

I think it is because once user use page down in a series of alt+L, the self.pos is not updated correctly, it probably is nil or zero, so "previous bookmark" Alt+K will have no effect, and "next bookmark" will jump to the first bookmark.

function CREReader:nextBookMarkedPage() for k,v in ipairs(self.bookmarks) do if self.pos < self.doc:getPosFromXPointer(v.page) then return v end end return nil end

dracodoc commented 9 years ago

@houqp , do you have time to have a look at this? This should be a simple problem. There is even comment in code mentioned that self.pos not updated in jumping history.

hwhw commented 9 years ago

Are you sure that self.pos is not updated correctly? The comment refers to making use of the (current) value of self.pos for jump history writing, later in the code (https://github.com/koreader/kindlepdfviewer/blob/master/crereader.lua#L288) self.pos is updated.

dracodoc commented 9 years ago

I knew it looks updated in the code, but I'm not sure if that part get executed in the special case -- using alt+K/L to jump bookmarks, then press page down. It sure can explain the behavior after press page down in bookmark jumping.

I could try to debug or track its value later.

houqp commented 9 years ago

sorry i got sick while travelling back home couple days ago. i will try take a look at it when i get better :(

dracodoc commented 9 years ago

No problem, you don't have to hurry at all! Take care :)

houqp commented 9 years ago

@dracodoc , sorry for the delay. I checked the code a bit and self.pos seems to be updated properly. Could you verify whether this bug exists in page mode as well?

dracodoc commented 9 years ago

@houqp , I verified page mode, the same bug exists in page mode. I guess very few people use the keyboard version of kpv now, even less people use the bookmark jumping feature in epub. If you don't have much time and the bug is not obvious (I though it should be relatively obvious), this is not a high priority task. I can avoid the bug with some effort because I know now I should not press page down in bookmark jumping :P

houqp commented 9 years ago

I will have to use my DXG sometime anyway. We either fix this bug or port koreader to DXG. Last time I tried it, I got stuck at the toolchain. Let's see if I can get it right this time ;)

dracodoc commented 9 years ago

@houqp , if you started to use your DXG, one of the more critical bug is that sometimes kpv crashes after a while into sleep and all the bookmarks and history for current opened book will be lost. Recently I had several crashes, I'm not sure the cause, but at least we can make sure that kpv write the history and bookmark information each time when user put the reader into sleep.

houqp commented 9 years ago

@dracodoc I cannot think of an easy way to achieve this in the old codebase. But we can do a dirty workaround: save page settings on every page turn.

Here is how to make it work: find the call back function for KEY_PGBCK and KEY_PGFWD key press in unireader.lua(line 2647) and crereader.lua. At the end of that function add following lines:

self:saveLastPageOrPos()
self.settings:flush()
NiLuJe commented 9 years ago

@houqp: Feel free to ping me if you hit a snag with TC issues ;).