EPUB navigator: internal links replace the whole web view context

[mihai-wolfpack]

Bug Report

What happened?

When opening an epub which has a table of contents inside the publication, taping on any of the links takes me to the correct location, however the currentLocator observer is not triggered and the progression of the publication is altered, navigating one page back takes me to the first page in the publication, and navigating forward restarts the publication from page 1 after the selected chapter from the ToC has endend. It was observed that on iOS this does not happen and there the behaviour is normal.

Expected behavior

When taping on a hyperlink of an entry in the ToC, the locator observer should be triggered and the progression / page order of the publication should not be altered in any way.

How to reproduce?

Inconjurat_de_idioti.epub.zip

1. Open the publication with an observer set on the currentLocator of the navigator
2. Tap on any of the hyperlinks in the ToC
3. Observe that the locator observer is not called
4. Observe that the page progression is altered when navigating back and forth

Environment

• Readium version: 2.1.0

Development environment

• OS: macOS 11.2
• IDE: Android Studio Arctic Fox 2020.3.1 Patch 2

Testing

• Android version: 10

• Model: OnePlus 5T

• Is it an emulator? No

Additional context

• Are you willing to fix the problem and contribute a pull request? Yes

[stevenzeck]

Yeah that's not great. The link is simply changing the URL of the webview that is in the current fragment to be that of the resource you selected, which replaces the contents of it entirely. In this case, the table of contents is completely replaced by whatever chapter you select.

Probably have to intercept the request in shouldOverrideUrlLoading (or this one) and ask the navigatorFragment to go to the resource.

[mihai-wolfpack]

I tried looking around to check whether there is any way to intercept the request and use the go method instead of it replacing the current URL bu unfortunately i was unable to find a propper solution. Could you please be a little bit more specific or provide such and example?

[mickael-menu]

Yeah internal links are very broken currently in the EPUB navigator.

You had the right approach, I think this should be fixed here, like Steven mentioned:

kotlin-toolkit/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt

Line 478 in 3670f7c

internal fun shouldOverrideUrlLoading(request: WebResourceRequest): Boolean {

Taking the Swift implementation as an example, we need to check if the link is internal and remove the base URL part to get only the href relative to the publication root. For example http://localhost:3842/uuid/chapt01.xhtml -> /chapt01.xhtml.

https://github.com/readium/r2-navigator-swift/blob/ad94c0c84662decd7f97e941b15e318d92263723/r2-navigator-swift/EPUB/EPUBSpreadView.swift#L436-L441

Then you can use this newly created href to create a Link object to be used with go():

https://github.com/readium/r2-navigator-swift/blob/ad94c0c84662decd7f97e941b15e318d92263723/r2-navigator-swift/EPUB/EPUBNavigatorViewController.swift#L727

[mihai-wolfpack]

Okay cool, I forked the repository and will open a PR as soon as i manage to fix it. Is this ok?

[mickael-menu]

Sure, thank you! Don't forget to check the "Allow edits from maintainers" when creating the PR, so we can edit the changelog or do small changes.

[mihai-wolfpack]

I have created a pull request here

[mickael-menu]

Thanks! I'll review this Friday

[mihai-wolfpack]

Hello there!
Any news regarding this? :D

[mickael-menu]

Yeah I'm actually working on this right now, there was an issue with lost anchors so we were only loading the first page of the target resource.

(I haven't try pushing on your branch yet, but did you enable edit from maintainers on it?)

[mihai-wolfpack]

Yes, it is enabled :D

[mihai-wolfpack]

I also have one more question regarding this. So the PR looks good with the changes provided by you, it can be merged from my side.
My question is the following: How and when will these changes be available for the app we are currently developing?

[mickael-menu]

I want to do a 2.1.1 release with these changes this week. As long as you are using the kotlin-toolkit monorepo in your project instead of the legacy repos, you should be able to upgrade.

[mihai-wolfpack]

That sounds amazing! Thank you very much :D