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

build: Fix Kobo poetry build compatibility #511

Merged
merged 1 commit into from Apr 25, 2022

Conversation

weijia-cheng
Copy link
Member

I spent today working on this and I think I have a satisfying fix for the Kobo poetry build issue. The code boils down to this:

  1. Mark all of the original poetry verse spans with the kobo-verse-span. The xpath uses the predicate [not(text()[normalize-space() != ''])] to distinguish ps that represent stanzas from prose ps . The idea behind this is that a stanza p should not have any text nodes as its children since all of the text will be in the spans.
  2. Update the poetry CSS so that the rules that target generic spans inside verses target spans with the class kobo-verse-span.

Here's an example of a de Cleyre poem with a broken header and footer before the fix:

Screenshot 2022-04-21 at 19-54-30 Poetry

This is the same poem with the fix:

Screenshot 2022-04-21 at 19-56-40 Poetry

Here is an example of a broken Newman epigraph:

Screenshot 2022-04-21 at 20-02-09 Verses on Various Occasions

This is the fixed epigraph:

Screenshot 2022-04-21 at 20-04-26 Verses on Various Occasions

This also plays well when you have a verse epigraph for a poem.

<header>
  <hgroup>
    <h2 class="first-child" epub:type="ordinal z3998:roman"><span id="kobo.782.1" class="koboSpan">VIII</span></h2>
    <h3 epub:type="title"><span id="kobo.783.1" class="koboSpan">The Trance of Time</span></h3>
  </hgroup>
  <blockquote class="epub-type-epigraph epub-type-z3998-verse xml-lang" lang="la" role="doc-epigraph" epub:type="z3998:verse epigraph" xml:lang="la">
    <p class="first-child">
      <span class="kobo-verse-span koboSpan" id="kobo.785.1">“Felix, qui potuit rerum cognoscere causas,</span>
      <br/>
      <span class="kobo-verse-span koboSpan" id="kobo.787.1">Atque metus omnes, et inexorabile fatum</span>
      <br/>
      <span class="kobo-verse-span koboSpan" id="kobo.789.1">Subjecit pedibus, strepitumque Acherontis avari!”</span>
    </p>
  </blockquote>
</header>

Here is an example showing how we aren't adding the kobo-verse-span class to a non-verse span (the Roman numeral at the end):

<header>
  <hgroup>
    <h2 epub:type="ordinal z3998:roman">CXXII</h2>
    <h3 epub:type="title">Waiting for the Morning</h3>
  </hgroup>
  <blockquote epub:type="epigraph">
    <p xml:lang="la">“Quoddam quasi pratum, in quo animae nihil patiebantur, sed manebant, nondum idoneae Visioni Beatae.”</p>
    <cite>
      <i epub:type="se:name.publication.book">Bedae <abbr>Hist.</abbr></i>
      <span epub:type="z3998:roman">V</span>
    </cite>
  </blockquote>
</header>

vs.

<header>
  <hgroup>
    <h2 class="first-child" epub:type="ordinal z3998:roman"><span id="kobo.6262.1" class="koboSpan">CXXII</span></h2>
    <h3 epub:type="title"><span id="kobo.6263.1" class="koboSpan">Waiting for the Morning</span></h3>
  </hgroup>
  <blockquote class="epub-type-epigraph" role="doc-epigraph" epub:type="epigraph">
    <p class="first-child xml-lang" lang="la" xml:lang="la"><span id="kobo.6265.1" class="koboSpan">“Quoddam quasi pratum, in quo animae nihil patiebantur, sed manebant, nondum idoneae Visioni Beatae.”</span></p>
    <cite>
      <i epub:type="se:name.publication.book"><span id="kobo.6266.1" class="koboSpan">Bedae </span><abbr><span id="kobo.6266.2" class="koboSpan">Hist.</span></abbr></i>
      <span epub:type="z3998:roman" id="kobo.6268.1" class="koboSpan">V</span>
    </cite>
  </blockquote>
</header>

I'm happy to check out more edge cases but I feel pretty confident in this.

@acabal
Copy link
Member

acabal commented Apr 22, 2022

That's great! Are you able to test on an actual Kobo to make sure the ebook works correctly? IIRC the spans are there to enable user highlighting/notetaking/dictionary lookups, so if those work on a Kobo with this new process then that's good.

Did you test any other ebooks in the corpus? What I would suggest is using the se compare-versions tool, which was made just for this. It requires Firefox. Basically, you take a known good repo, then apply some changes to it, leaving it in a dirty state. Then run se compare-versions on the repo, which will use Firefox to take screenshots of the ebook in its clean state, then in its dirty state, and generate a visual diff of the two states. If there's no diff then that means the ebooks render identically in both states, which is what we want. If there is a diff then the new code does something unexpected to the rendering and you can explore the situation to figure out why.

Some ebooks to try this on would be Keats, Frost, Carroll.

@weijia-cheng
Copy link
Member Author

This has a demonstration of the rendering on a Kobo.

I didn't know about compare-versions! That does seem like a good test for this. I'll play with that tool later today and run this test with de Cleyre, Newman, Keats, Frost, and Carroll.

@acabal
Copy link
Member

acabal commented Apr 22, 2022

This has a demonstration of the rendering on a Kobo.

Is that eink? Does highlighting/dictionary work?

@weijia-cheng
Copy link
Member Author

Yes, it is eink and the highlighting and dictionary work as I demonstrate on the last few pages. (I used a scanning app on my phone to take the pictures, which applied a black and white filter to some of the images.)

@acabal
Copy link
Member

acabal commented Apr 22, 2022

Great, thanks. Let me know if compare-versions comes up clean.

@weijia-cheng
Copy link
Member Author

Ok, so I'm doing the compare-versions test now. My testing procedure is:

  1. Build and extract compatible and Kobo versions of the ebook
  2. Copy the extracted compatible epub files into the repository and create a test commit
  3. Copy the extracted Kobo files into the repository
  4. Run compare-versions to compare the Kobo version against the compatible version
wcheng@pop-os:~/git/voltairine-de-cleyre_poetry$ se compare-versions .
Difference in endnotes.xhtml
wcheng@pop-os:~/git/john-henry-newman_verses-on-various-occasions$ se compare-versions .
Difference in endnotes.xhtml
wcheng@pop-os:~/git/john-keats_poetry$ se compare-versions .
Difference in endnotes.xhtml
wcheng@pop-os:~/git/robert-frost_north-of-boston$ se compare-versions .

I gave up on Carroll because that repo is huge and slow to build, and there isn't that much poetry in there anyways. All of the endnote differences are expected (it's just the different backlink character used for Kobo). The reason I tested against the compatible epub build was that I did notice an unrelated existing issue with footers and CSS specificity with the compatibility epub while building de Cleyre. I'm going to file a separate issue for that.

@acabal
Copy link
Member

acabal commented Apr 25, 2022

OK, fantastic work Weijia. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants