search

readium / readium-js-viewer

👁 ReadiumJS viewer: default web app for Readium.js library
BSD 3-Clause "New" or "Revised" License
550 stars 186 forks source link

Users of Chrome app report that 0.29 does not retain reading position. #683

Closed rkwright closed 6 years ago

rkwright commented 6 years ago

This issue is a Bug

Related issue(s) and/or pull request(s)

None

Expected Behaviour

Chrome app should restore user's reading position at start of new session

Observed behaviour

Instead, users report that it reverts to start of book

Steps to reproduce - as reported by users.

  1. Open book
  2. Navigate to some point in the book.
  3. Close app.
  4. Restart app.
  5. Book opens at beginning of book instead of saved position

Test file(s)

None

Note: RKW unable to reproduce this on OSX 10.10.5

Product

Additional information

None yet

rkwright commented 6 years ago

@danielweck I can't reproduce this on OSX. Can you try it on Windows? I don't have a working Windows box at the moment.

rkwright commented 6 years ago

@danielweck Remembered that my headless Weather station server is Windows. Logged in and ran the Chrome app there with a feedbook download. Sure enough, it goes back to first page every time. This is a Windows-specific bug.

rkwright commented 6 years ago

More data. Tried the firebaseapp site (which has 0.30-alpha) and that works fine. Also tried readium.surge.sh - which is 0.29 master and it works fine too. So this appears to be a Windows Chrome app specific failure.

rkwright commented 6 years ago

Actually, it now appears that the problem appeared in 0.28 and we didn't catch it. There is one report from a user on Dec 19 - before we pushed the 0.29 app. Yikes.

danielweck commented 6 years ago

I am able to reproduce this bug on Windows 10.

danielweck commented 6 years ago

Actually, I can reproduce this on OSX as well. For example, open an EPUB, go to another spine item document / HTML "chapter" (by turning pages, or via a direct TOC hyperlink), go back to the library view, re-open the same EPUB, and observe how the newly-navigated spine item isn't restored. That is because the old value of the "saved reading location" is used from local storage, or if there is none: the beginning of the spine is loaded.

Culprit: the savePlace() function is called, but because the epub URL querystring parameter is not present, the function body exits prematurely:

https://github.com/readium/readium-js-viewer/blob/develop/src/js/EpubReader.js#L762

The breaking code change was introduced here (back in July 2017):

https://github.com/readium/readium-js-viewer/commit/21d270b5b6625c5733746ebad44719edf2569b57#diff-7edd3ba57a8b43c2180f11f16a240a06

@JCCR We tested the cloud reader heavily but we didn't notice the breakage in the Chrome app. Let's try to fix this for the next release!

danielweck commented 6 years ago

This should fix it: https://github.com/readium/readium-js-viewer/commit/e08101821d8113fe15428e4ae3137e2a07b13c41 (testing...)

danielweck commented 6 years ago

The fix works fine in my local build of the Chrome app. I will test the TravisCI build of the cloud reader...

danielweck commented 6 years ago

Cloud reader works fine too:

https://travis-ci.org/readium/readium-js-viewer/builds/330518618

=>

https://readium.firebaseapp.com/?epub=epub_content%2Faccessible_epub_3&goto=epubcfi(/6/18!/4/2/26/1:0)

danielweck commented 6 years ago

Fixed. Closing.

lucasavila00 commented 6 years ago

Hi! Thanks for your awesome work on this project! I'm having this issue on my chromebook using the chrome webstore extension. My local version is 2.29.0 (built at December 22 it seems) and it appears that it hasn't received the fix you've made. Do you have any release schedule for the extension?

Anyway, I'd like to hear back from you to decide between waiting for the upgrade to hit Chrome Store or if I should build the latest version and use it for now as this feature is a must for me.

Again, thanks for your hard work 👍

danielweck commented 6 years ago

Until the next Chrome app release, you can build your own from readium-js-viewer using the command line: 1) git clone --recursive -b develop https://github.com/readium/readium-js-viewer.git readium-js-viewer 2) cd readium-js-viewer 3) git submodule update --init --recursive 4) git checkout develop && git submodule foreach --recursive "git checkout develop" 5) npm run prepare:all 6) npm run dist+sourcemap 7) load the app from the ./build/chrome-app/ folder, with Chrome in developer mode.