fix: properly scroll vertical text #656

shovel-kun · 2025-05-04T10:53:06Z

Fixes scrolling vertical RTL epubs. See before and after video.

EPUB used in demo: https://www.gutenberg.org/ebooks/31617

BEFORE

before.mp4

AFTER

after.webm

…epub-rtl-scroll

Copilot

Pull Request Overview

This PR fixes the scrolling issue for vertical RTL EPUBs by introducing overscroll detection in the WebView and adjusting scrolling logic in JavaScript for vertical writing mode.

Introduces a new overscroll mode in the WebView to determine the proper scroll direction.
Adds a new utility function to detect vertical writing mode and adjusts the scroll behavior accordingly in JavaScript.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
readium/navigator/src/main/java/org/readium/r2/navigator/R2WebView.kt	Adds overscroll mode detection and updates touch event handling for scrolling.
readium/navigator/src/main/assets/_scripts/src/utils.js	Introduces vertical writing mode detection and adjusts scroll-to-start/end logic accordingly.

Comments suppressed due to low confidence (1)

readium/navigator/src/main/java/org/readium/r2/navigator/R2WebView.kt:749

[nitpick] Consider extracting the magic number '200' into a well-named constant to clarify its purpose and improve maintainability.

if (abs(totalDelta) < 200) {

readium/navigator/src/main/assets/_scripts/src/utils.js

mickael-menu

Thank you @shovel-kun!

It's looking great in vertical RTL mode, but I noticed a regression in LTR vertical. When going backward to the previous chapter, it's scrolled to the top instead of the bottom/end. Would you mind taking a look? Thank you.

I also simplified a bit the JS conditionals but the issue was there before.

shovel-kun · 2025-05-15T19:21:30Z

@mickael-menu Thanks for reviewing my PR!

I also discovered that currentLocator do not update on scroll in vertical RTL mode, so I'll have to fix that too in addition to the issue you mentioned.

mickael-menu · 2025-05-16T08:09:33Z

Ha yes, I had this issue on my radar but I haven't had the time to address it yet. I appreciate your willingness to take a look!

shovel-kun · 2025-05-16T09:31:46Z

Hi @mickael-menu, I fixed the regression for LTR vertical.

However, I notice that the writing-mode will always be vertical-rl in LTR, when it should be vertical-lr (tested on JP epub and EN epub) The cause is this line in ReadiumCSS-pagination-vertical.css.

I'd like your advice on how to resolve this issue, since I'm unfamiliar with how Readium handles stylesheets.

mickael-menu · 2025-05-16T10:01:12Z

Thank you,

Could you open an issue on https://github.com/readium/css for this other issue? Jiminy should help you out and then we can import the fixed stylesheets in the toolkit.

shovel-kun · 2025-05-17T07:08:15Z

@mickael-menu Will do!

By the way, are you sure the issue you described before is a regression? I am able to replicate it on develop

Steps to reproduce:

In TestApp, open an Epub.
Enable scroll LTR.
Scroll and confirm it pages correctly.
Change to scroll RTL
It does not page correctly

The issue is caused by the SimpleOnPageChangeListener not being readded to resourcePager when resourcePager is invalidated.

I've patched this by creating a private member var to store pageChangeListener, and readding the listener on every invalidation.

Shall I open a new PR or should I include the patch in this PR?

Subject: [PATCH] stash
---
Index: readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt
--- a/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt	(revision c1f62fe5aa4170bf59d82b636f324f167b6a0548)
+++ b/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt	(date 1747465013631)
@@ -285,6 +286,8 @@
 
     private var state: State = State.Initializing
 
+    private var pageChangeListener: ViewPager.OnPageChangeListener? = null
+
     // Configurable
 
     override val settings: StateFlow<EpubSettings> get() = viewModel.settings
@@ -412,7 +415,7 @@
         resourcePager = binding.resourcePager
         resetResourcePager()
 
-        resourcePager.addOnPageChangeListener(object : ViewPager.SimpleOnPageChangeListener() {
+        pageChangeListener = object : ViewPager.SimpleOnPageChangeListener() {
 
             override fun onPageSelected(position: Int) {
 //                if (viewModel.layout == EpubLayout.REFLOWABLE) {
@@ -422,6 +425,7 @@
                     if (viewModel.isScrollEnabled.value) {
                         if (currentPagerPosition < position) {
                             // handle swipe LEFT
                             webView.scrollToStart()
                         } else if (currentPagerPosition > position) {
                             // handle swipe RIGHT
@@ -430,6 +434,7 @@
                     } else {
                         if (currentPagerPosition < position) {
                             // handle swipe LEFT
                             webView.setCurrentItem(0, false)
                         } else if (currentPagerPosition > position) {
                             // handle swipe RIGHT
@@ -441,7 +446,10 @@
 
                 notifyCurrentLocation()
             }
-        })
+        }
+
+        // should we throw if not found?
+        pageChangeListener?.let { resourcePager.addOnPageChangeListener(it) }
 
         // Fixed layout publications cannot intercept JS events yet.
         if (publication.metadata.presentation.layout == EpubLayout.FIXED) {
@@ -554,7 +562,9 @@
 
     private fun invalidateResourcePager() {
         val locator = currentLocator.value
+        val listener = pageChangeListener
         resetResourcePager()
+        listener?.let { resourcePager.addOnPageChangeListener(it) }
         go(locator)
     }

mickael-menu · 2025-05-17T07:16:19Z

You're right, it looks like the issue only happened after changing the scroll or reading progression preference in the same reading session. Nice catch! Feel free to add your patch to this PR. I'll review the whole PR again next week. Thanks again!

shovel-kun · 2025-05-17T13:17:40Z

Fixed progression not updating on scroll in Vertical Text mode.

Vertical LTR chapter paging is still broken, since I am unable to get the user's desired ReadingProgression from native to webview. The properties dir and writing-mode are unreliable because dir has been forbidden from applying in vertical text, and writing-mode is always vertical-rl in the CJK vertical text stylesheet. I am waiting for readium/css#180 to be resolved in order to fix this.

I also noticed that highlighting in vertical text is broken (the decorations rectangle are incorrectly positioned). I might try and tackle this too, unless there other considerations?

Copilot

Pull Request Overview

This PR fixes vertical text scrolling in EPUBs with right‐to‐left reading progression by propagating vertical text settings across the navigator components and adjusting overscroll and progression calculations accordingly.

Added a verticalText property in the view model and propagated it to the fragment and web views.
Updated scroll and progression logic in R2WebView.kt, R2BasicWebView.kt, and utils.js to handle vertical writing modes.
Updated CHANGELOG.md to reflect the fixes.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
EpubNavigatorViewModel.kt	Added verticalText getter to expose vertical text setting.
EpubNavigatorFragment.kt	Introduced pageChangeListener variable and propagated verticalText to the layout.
R2WebView.kt	Implemented overscroll mode logic and adjusted scrolling behavior for vertical text.
R2BasicWebView.kt	Updated progression calculation to handle vertical text scrolling.
utils.js	Added vertical writing mode detection and adjusted scroll position calculations accordingly.
CHANGELOG.md	Documented the vertical text scrolling fixes.

readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt

readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt

readium/navigator/src/main/assets/_scripts/src/utils.js

mickael-menu

Thank you, this is very helpful!

Vertical LTR chapter paging is still broken, since I am unable to get the user's desired ReadingProgression from native to webview. The properties dir and writing-mode are unreliable because dir has been forbidden from applying in vertical text, and writing-mode is always vertical-rl in the CJK vertical text stylesheet. I am waiting for readium/css#180 to be resolved in order to fix this.

Yeah I think we should just disable LTR reading progression when vertical = true for now, until we upgrade to Readium CSS and the option is available.

I also noticed that highlighting in vertical text is broken (the decorations rectangle are incorrectly positioned). I might try and tackle this too, unless there other considerations?

No feel free to contribute the fix if you need. We don't have anyone on our team experienced in vertical reading, so proper feedback from implementers is very helpful.

Thanks again.

shovel-kun and others added 2 commits May 4, 2025 18:20

fix: properly scroll vertical text

4d921f4

chore: remove commented out logs

6664186

shovel-kun mentioned this pull request May 4, 2025

[Bug] Chinese epub vertical documents cannot be read normally #458

Open

shovel-kun and others added 4 commits May 4, 2025 22:42

readd Timber

66aadef

Merge branch 'develop' of github.com:readium/kotlin-toolkit into fix/…

37faaef

…epub-rtl-scroll

Merge branch 'develop' into fix/epub-rtl-scroll

9db3820

Simplify JS

a10751c

mickael-menu requested a review from Copilot May 15, 2025 09:24

Copilot AI reviewed May 15, 2025

View reviewed changes

readium/navigator/src/main/assets/_scripts/src/utils.js Outdated Show resolved Hide resolved

mickael-menu reviewed May 15, 2025

View reviewed changes

Merge branch 'readium:develop' into fix/epub-rtl-scroll

9f78e08

fix regression for vertical ltr

da3ad4f

fix: readd listener on pager invalidation

d02e21c

shovel-kun mentioned this pull request May 17, 2025

How override vertical-rl from ReadiumCSS-pagination-vertical.css with vertical-lr readium/css#180

Open

mickael-menu added 2 commits May 20, 2025 11:56

Clean up and lint

2f31539

Update changelog

dafad14

mickael-menu requested a review from Copilot May 20, 2025 09:59

Copilot AI reviewed May 20, 2025

View reviewed changes

Refactoring

d973ec8

mickael-menu approved these changes May 20, 2025

View reviewed changes

mickael-menu merged commit e5f5ad1 into readium:develop May 20, 2025
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: properly scroll vertical text #656

fix: properly scroll vertical text #656

shovel-kun commented May 4, 2025

Copilot AI left a comment

mickael-menu left a comment

shovel-kun commented May 15, 2025

mickael-menu commented May 16, 2025

shovel-kun commented May 16, 2025

mickael-menu commented May 16, 2025

shovel-kun commented May 17, 2025 •

edited

Loading

mickael-menu commented May 17, 2025

shovel-kun commented May 17, 2025

Copilot AI left a comment

mickael-menu left a comment

fix: properly scroll vertical text #656

fix: properly scroll vertical text #656

Conversation

shovel-kun commented May 4, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

mickael-menu left a comment

Choose a reason for hiding this comment

shovel-kun commented May 15, 2025

mickael-menu commented May 16, 2025

shovel-kun commented May 16, 2025

mickael-menu commented May 16, 2025

shovel-kun commented May 17, 2025 • edited Loading

mickael-menu commented May 17, 2025

shovel-kun commented May 17, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

mickael-menu left a comment

Choose a reason for hiding this comment

shovel-kun commented May 17, 2025 •

edited

Loading