-
Notifications
You must be signed in to change notification settings - Fork 3
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
question about html root element #14
Comments
Yes it looks like you are correct. Combining this with issue #10 it seems that maybe the correct behavior is to always ignore the root element, whether it's a |
Yes, that makes sense to me. Would you like me to work on that and make a merge request? |
If you have the spoons that would be great. I'm traveling and might not be very responsive for the next few days. |
No problem |
I made the pull request: #15 |
This may be somewhat related to issue #10 .
The epub cfi spec does not indicate what the root element of the html portion of the cfi should be. However, in all the examples, the body element is assigned the cfi step reference number of
4
.This would seem to indicate that root
html
element is not included in the step reference.That also agrees with your tests found in https://github.com/fread-ink/epub-cfi-resolver/blob/master/tests/simple.js#L120-L251.
In those tests, all the html step references begin with
4/
.The issue is that the cfi generator code behaves differently. It includes the root
html
element in the step reference and adds the leading 2:2/4...
For example in the generator.js test you can see that the test string includes the leading
2
:epub-cfi-resolver/tests/generator.js
Line 54 in 66f0543
I believe the correct behavior would be for the generator to stop traversing the DOM tree when it reaches the
body
elements.If you agree, I would be happy to make a pull request.
The text was updated successfully, but these errors were encountered: