fread-ink / epub-cfi-resolver

A simple parser, resolver and generator for the EPUB-CFI format
GNU Affero General Public License v3.0
19 stars 3 forks source link

question about html root element #14

Open halmos opened 2 years ago

halmos commented 2 years ago

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: https://github.com/fread-ink/epub-cfi-resolver/blob/66f054365c8700abcc64fb2bad71a5156cddb23c/tests/generator.js#L54

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.

Juul commented 2 years ago

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 html element or a package element or something else. Does that seem right to you? If so then perhaps we can replace your fix for issue #10 with a generic "skip the root element" fix?

halmos commented 2 years ago

Yes, that makes sense to me. Would you like me to work on that and make a merge request?

Juul commented 2 years ago

If you have the spoons that would be great. I'm traveling and might not be very responsive for the next few days.

halmos commented 2 years ago

No problem

halmos commented 2 years ago

I made the pull request: https://github.com/fread-ink/epub-cfi-resolver/pull/15