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

Severe bug in Chrome app: links fail when spine items with href dot prefix `./path/filename.ext` #672

Closed danielweck closed 6 years ago

danielweck commented 6 years ago

Spine items paths fail to match during hyperlinking due to URI.js URN syntax parsing (percent escape encoding, vs. path normalization based on slashes). This affects linking from the TOC, as well as from one spine item document to another. This is because the Chrome app uses the filesystem:chrome-extension://UUID/path/to/filename.ext URL syntax, which is parsed as URN with filesystem protocol by URI.js (see diff link below for historical patch of URI.js's absoluteTo() function: https://github.com/medialize/URI.js/compare/master...danielweck:master ). Unfortunately the path normalization step fails to remove ./ prefix from spine item hrefs due to slashes being replaces with percent escapes by the URI.js lib. The obvious affected ReadiumJS function is Spine.getItemByHref(), but there may be others: https://github.com/readium/readium-shared-js/blob/26d6eb1d70134b9bfb490d228168d6bfeac9c747/js/models/spine.js#L287-L303

danielweck commented 6 years ago

Fixed in https://github.com/readium/readium-shared-js/commit/d9eca1d980ed4bef636548b7e7d7fd9f0cde7d5a#diff-3b43f795d83fc0addc67ec68c3864a11 Tested in actual Chrome app with (private) EPUB that exhibited the original bug. I will check other usages of URI() in readium-js and readium-js-viewer before closing this issue.

danielweck commented 6 years ago

I checked usages of new URI() in the entire ReadiumJS repository and I am quite confident that this URI.js path normalization issue is not triggered elsewhere. Closing.

danielweck commented 6 years ago

Update: also successfully re-tested the web / cloud reader, which wasn't originally affected but I wanted to check for regression bugs: https://readium.firebaseapp.com