Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTL Hebrew books do not render properly - columns are split #282

Open
rkwright opened this issue Apr 8, 2016 · 40 comments
Open

RTL Hebrew books do not render properly - columns are split #282

rkwright opened this issue Apr 8, 2016 · 40 comments

Comments

@rkwright
Copy link
Member

rkwright commented Apr 8, 2016

This is a BUG report

Reported on the IDPF forum by hagiler: http://idpf.org/forum/topic-3020

"I design hebrew e-books for several clients, usually using their own app for books. However, one client wanted to test the book on Readium and found out that the text looks weird: http://imgur.com/I8yd4s8
Note how half of the second page moves to the right of the first page.

I've tried it myself and got the same result, even with books that used to work before. I've also tested it with several english epubs which seemed to work fine, so I'm pretty sure it's only in Hebrew.

Did anything change in the last few versions that might have caused this? I can provide a sample book if needed."

Expected:
Hebrew text will be laid out properly.

Result:
Text is sometimes broken across three columns.

image

Steps to reproduce.

  1. Load the test book (requested, pending)
  2. Page through it
  3. Sometimes the page is broken across 3 columns.

Browser/OS:
Not known yet.

@rkwright
Copy link
Member Author

rkwright commented Apr 8, 2016

It appears that we have a fix for this, but we are seeing a different problem (but similar, in Arabic). We are looking at the other problem now.

@rkwright
Copy link
Member Author

@danielweck
Copy link
Member

@danielweck
Copy link
Member

@jccr there's a CFI -related message in the web console. Would you mind taking a look?

@danielweck
Copy link
Member

@jccr in that same ebook ( https://github.com/readium/readium-test-files/blob/master/issue-files/shared-js/282/Hashlachot_Getbooks.epub ), there's a "severe" CFI error:

Uncaught IndexSizeError: Failed to execute 'setEnd' on 'Range': The offset 14 is larger than or equal to the node's length (13)
at cfi_navigation_logic.js line 914

Press the left arrow (i.e. next page) and you should see an infinite loader / spinner:

https://readium.firebaseapp.com/?epub=https%3A%2F%2Frawgit.com%2Freadium%2Freadium-test-files%2Fmaster%2Fissue-files%2Fshared-js%2F282%2FHashlachot_Getbooks.epub&goto=%7B%22idref%22%3A%22Hashlachot_Getbooks-1%22%2C%22elementCfi%22%3A%22%2F4%5BHashlachot_Getbooks-1%5D%2F2%2F2%2F2%2C%2F1%3A0%2C%2F1%3A1%22%7D

@mahag
Copy link

mahag commented May 17, 2016

I did run my tests over several browsers and operating systems.
Google chrome fails on most of Os :windows 7 ,windows 8,windows 10 ,ios(google chrome app in iphone/ipad)
IE 11and edge worked correctly under windows
Safari in IOS worked too
So this might be related to something in google chrome

Any updates?

@danielweck
Copy link
Member

thanks @mahag, for your tests.
no development news on that front as far as I can tell.

@danielweck
Copy link
Member

Related: readium/readium-js-viewer#516

@danielweck
Copy link
Member

@danielweck
Copy link
Member

Helicon's dev says that this fixes the bug (which ; unfortunately ; I am not able to reproduce):

https://github.com/readium/readium-shared-js/blob/develop/js/views/reflowable_view.js#L779

@ -45905,9 +45919,15 @@ var ReflowableView = function(options, reader){

     function showBook()
     {
+        console.log("PATCHED by OZ");
+        var currOpacity = _currentOpacity;
         if (_currentOpacity != -1)
         {
-            _$epubHtml.css('opacity', _currentOpacity);
+            _$epubHtml.hide();
+            setTimeout(function() {
+                _$epubHtml.show();
+                _$epubHtml.css('opacity', currOpacity);
+            }, 50);
         }
         _currentOpacity = -1;
     }

@mahag
Copy link

mahag commented Sep 21, 2016

Was this solved for arabic books ?

@rkwright
Copy link
Member Author

@mahag We believe so, but the bug is hard to reproduce. Can you reproduce with the current build?

@mahag
Copy link

mahag commented Sep 22, 2016

I am trying to get the current build. I just redo the steps for branch name master correct?

@mahag
Copy link

mahag commented Sep 22, 2016

I did run the steps using master branch but am getting an error
untitled2

@mahag
Copy link

mahag commented Sep 22, 2016

I was able to download readium on a different machine and it worked. I place the cloud-reader-lite folder on my server and placed the epub in it and opened the url in google chrome i got the same problem

untitled4

@rkwright
Copy link
Member Author

@danielweck Looking at the code in reflowable_view.js (in develop) it does not appear that the patch from Helicon was ever merged in and/or tested? Is that a correct statement?

@mahag
Copy link

mahag commented Sep 29, 2016

I did use master branch and not develop.Do i retry with develop?

@rkwright
Copy link
Member Author

@mahag Yes, please. Let us know what you find.

@mahag
Copy link

mahag commented Oct 3, 2016

I did retry develop and run my file in readium but still the issue occurs.
In developer tools there is an error , am not sure if it is related to the display.

Uncaught TypeError: Cannot read property 'off' of null
untitled1

And this is the error in console when running the master branch version

Discontiguous selection is not supported.

untitled2

In both tests I am using same epub file

@danielweck
Copy link
Member

Please build the cloud reader with source maps, so that you can debug into non-minified / uglified Javascript source code.
Use 'npm run prepare && npm run dist+sourcemap' (the resulting app in the "dist/cloud-reader-lite" folder will contain the additional source maps (large .map files, web browsers only load them when debugging from the web inspector).

@mahag
Copy link

mahag commented Oct 3, 2016

This is what i got
untitled8

I noticed that once a new chapter opens and we flip pages to read it this wrong display occurs while if go back to it (flipping in the opposite direction) as if reading it backwards from last page to 1st page the display is correct.

@looknear
Copy link

looknear commented Oct 3, 2016

similar thing happen on Android RTL books
right margin is cut. but when you flip pages till last page in chapter - and then flip back - the pages are not cut anymore and located in the correct position. (till you open new chapter - and then its cut again)

@danielweck
Copy link
Member

Rangy is not used for anything critical in Readium (only a prototype feature which isn't actually used by content creators).
The console message is a warning that can be ignored.

@looknear
Copy link

looknear commented Oct 3, 2016

I see that in some cases when RTL issue is reported - you mention "helikon" version. is there an "Helicon" stream or branch were we can see their contribution to the code?

@danielweck
Copy link
Member

@looknear try this small patch in the showBook() function:
#282 (comment)

@looknear
Copy link

looknear commented Oct 4, 2016

Sorry for stupid newbie question - where you change it?
it is found in readium_shared_js_all (single and multi dirs) - which is overridden on compile with Android Studio back to default....
is there some architecture document or some development process tutorial?... (or i'm asking too much...:)) )
also - if helicon fixed it - where is their branch?... why to fix twice? when it will move into dev branch?

@danielweck
Copy link
Member

danielweck commented Oct 4, 2016

@looknear I strongly suggest that you build your own cloud reader app (including source maps so that you can debug into non-minified / uglified Javascript source code), in order to integrate your custom code, etc.
It's simple enough once you have NodeJS isntalled: run npm run prepare && npm run dist+sourcemap from your command line (the generated app will be in the dist/cloud-reader-lite folder).
Helicon's custom code is:
https://github.com/readium/readium-shared-js/blob/develop/js/views/reflowable_view.js#L783

@ -45905,9 +45919,15 @@ var ReflowableView = function(options, reader){

     function showBook()
     {
+        console.log("PATCHED by OZ");
+        var currOpacity = _currentOpacity;
         if (_currentOpacity != -1)
         {
-            _$epubHtml.css('opacity', _currentOpacity);
+            _$epubHtml.hide();
+            setTimeout(function() {
+                _$epubHtml.show();
+                _$epubHtml.css('opacity', currOpacity);
+            }, 50);
         }
         _currentOpacity = -1;
     }

@looknear
Copy link

looknear commented Oct 4, 2016

Thanks you. i managed to see the change in the app.
you are right. better testing on cloud as web app - on clean js... (hoping webview will give same result). will play with it some more. anyway - the patch fix the right cut margins..., thanks.

@looknear
Copy link

looknear commented Oct 4, 2016

by the way... the link you sent to the file - does not contain the above patch.

@danielweck
Copy link
Member

@looknear correct. The link points to the latest code in the develop branch, which does not contain Helicon's proposed patch. Could you please apply this simple code patch at your end, and check if it helps solve the RTL pagination issues? Many thanks!

@looknear
Copy link

looknear commented Oct 4, 2016

it solve the right margin text cut. will check the column pagination issue
when will manage to activate a local web viewer.

On Tue, Oct 4, 2016 at 7:56 PM Daniel Weck [email protected] wrote:

@looknear https://github.com/looknear correct. The link points to the
latest code in the develop branch, which does not contain Helicon's
proposed patch. Could you please apply this simple code patch at your end,
and check if it helps solve the RTL pagination issues? Many thanks!


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#282 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA91x6KR3eyIvp77dkdup6VrHleBdwQZks5qwoVTgaJpZM4ICzqa
.

@danielweck
Copy link
Member

You can start a local web server by invoking the command "npm run http" (this should automatically open a web browser with the http://127.0.0.1/dev/index.html URL). This will allow you to instantly debug updated code (no need to compile the "dist" app bundles, minified / optimised by RequireJS)
Alternatively, you can also test a real build of the app, by opening http://127.0.0.1/distribution/cloud-reader/index.html

@mahag
Copy link

mahag commented Oct 6, 2016

So how can i fix this display issue in readium js viewer for rtl page progressions?

@danielweck
Copy link
Member

Can you please try this?
#282 (comment)

@mahag
Copy link

mahag commented Oct 19, 2016

which file do i have to open to change showbook function? I tool the folder cloud reader lite from dist folder and placed it on my server. Where do i have to check after that?

@danielweck
Copy link
Member

Hello @mahag please follow the minimalist build instructions in my above comment:
#282 (comment)
...and of course the full README is available too:
https://github.com/readium/readium-js-viewer/blob/develop/README.md#development

@ghost
Copy link

ghost commented Jun 10, 2017

@danielweck Helicon's custom code with timeout trick makes it better but it's not a final solution. there is still some rendering problem in web kit based browsers like Chrome. Do you have any idea about this RTL books issue?
readium

@danielweck
Copy link
Member

Hello @samkeller I'm afraid none of us has had enough bandwidth to look further into RTL pagination issues.
Looks like a browser quirk too :(

@heidarv
Copy link

heidarv commented Oct 21, 2017

@danielweck I have the same issue in Arabic and Persian languages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants