Opening a new book does not reset the current page, possibly leading to a crash

[simbul]

Opening a new book does not reset the current page, possibly leading to a crash

[Xm4s]

Hi!

Thanks for pointing out this!
I've looked into this bug and the problem is in your commit request.

If you reset currPageNumber to 1 only when the saved value of currPageToLoad is 0 (which in my calculation was impossible even before your contribution)... well, the chance you are resetting the variable to 1 are equal to 0 :P

I'm fixing this with the bug pointed out in the issue #36.

[Xm4s]

Fixed with the commit b8a6d2e
As for issue #36 I'll wait for a positive feedback on the fix before closing this up.

[andreacampi]

Well, this is different than issues #37/#38. Reproducing the problem with page 0 is probably impossible with unmodified sources (barring any bugs).

This issue however is 100% reproducible, see the steps I outlined above.

[andreacampi]

Uhm, I saw you reverted the change that protects against currPageToLoad=0.

As I said, I agree it should never happen, but you shouldn't assume it never will.

Imagine the app was just started but we lose focus immediately (for instance, an SMS comes in) before we have even completed -initBook:
In that case, rootViewController.currentPageNumber will still be nil (it's not set in RootViewController:-init) so -saveLastPageReference will probably save lastPageViewed=0.
In that case, reading it back will get a 0 here.

Actually, in my commit I meant to protect again this particular scenario in -saveLastPageReference, but I botched it:

    if (lastPageViewed != 0) {

That's just plain wrong, sorry about that :( It should probably be:

if (rootViewController.currentPageNumber > 0) {

Anyway, the bottom line is, you should never trust that something will never happen; a bug in other parts of code may lead to this invariant not being true, and that will positively crash the app. I think you'll agree that's not nice :)

[Xm4s]

I completely agree with you, even if it shouldn't be possible it's better to reduce any chance of the app crashing down, and in fact I didn't revert your change on lastPageView value check before saving it :)

However, as you have said here, the check as it is need a quick fix, I'll apply that as soon as I finish writing this, thanks for pointing it out.

Back to the topic of this issue, I try the step to reproduce the bug (link on page 5 a .hpub with only 3 pages) and it seems that everything works pretty well with the last commit. Could you please confirm that?

[andreacampi]

Works for me, too. Thanks for looking at this :)

[Xm4s]

Thank you for your precious support on the project :D
I'm closing this up.