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

getCurrentSelectionCfi() error with following xhtml #47

Open
zoomiinc opened this issue Feb 5, 2016 · 8 comments
Open

getCurrentSelectionCfi() error with following xhtml #47

zoomiinc opened this issue Feb 5, 2016 · 8 comments

Comments

@zoomiinc
Copy link

zoomiinc commented Feb 5, 2016

Thanks in advance for looking at this.
When we're using the attached xhtml file (in a zip) we're getting this error when selecting a section that starts in the first paragraph and ends in the second paragraph:
Uncaught SyntaxError: Expected "!/", "/", "[" or [0-9] but "," found.

The CFI looks like for example like this before being passed to peg$parse:
epubcfi(/4,/4/1:16,/6/1:151)

Any idea how this could be fixed? Thanks in advance!

text-1.xhtml.zip

@danielweck
Copy link
Member

Thanks for the feedback.
What branch? (develop or master)
What Readium app? (Chrome extension, cloud/web reader, native launcher)
If cloud reader, what web browser?

@zoomiinc
Copy link
Author

zoomiinc commented Feb 5, 2016

Oh, I apologize for forgetting this. We're using the latest readium-js directly. Tried master and develop. Happens across all browsers (tested latest FF, Chrome, Safari)
Thanks in advance.

@danielweck
Copy link
Member

I assigned Juan because he is pretty good at figuring out CFI -related bugs, but that does not mean he has the required free cycles to work on this now :)

@jccr
Copy link
Member

jccr commented Feb 5, 2016

@iLern
Hi, thanks for the info. Could you share a screenshot with your selection, just to make sure I reproduce it the same way? Thanks.

@zoomiinc
Copy link
Author

zoomiinc commented Feb 5, 2016

@danielweck
Copy link
Member

@iLern you can add screenshots / images directly into the GitHub comment box :)
(I did that for you, see above)

@jdempcy
Copy link

jdempcy commented Feb 5, 2016

This seems to be a difference between the range CFI and the kind of CFI
expected by whatever method is throwing that error.

The correct fix would be to change the method to accept range CFIs (which
contain commas).

A quick fix, in the interim, would be to truncate the comma and everything
past it.

For instance:

var cfi = '/4,/4/1:16,/6/1:151';
var truncatedCfi = cfi.split(',')[0];

console.log(truncatedCfi); // '/4'

Jonah Dempcy
T 206.226.2857 | E [email protected]
Follow on Facebook https://www.facebook.com/jdempcy | @jdempcy
http://www.twitter.com/jdempcy | /in/jdempcy
http://htmlsandbox.com/www.linkedin.com/in/jdempcy

On Fri, Feb 5, 2016 at 11:45 AM, Daniel Weck [email protected]
wrote:

@iLern https://github.com/ilern you can add screenshots / images
directly into the GitHub comment box :)
(I did that for you, see above)


Reply to this email directly or view it on GitHub
#47 (comment)
.

@bradleygore
Copy link

I just posted a bug in the readium-shared-js repo as I didn't realize there was a separate CFI repo, but I think it's related to this issue. It has steps to reproduce on the readium firebase app as well as screenshots. Any help appreciated!

readium/readium-shared-js#313

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

5 participants