TEIC / Stylesheets

TEI XSL Stylesheets
228 stars 123 forks source link

wrap new att lists in `<ul>` (where they were not so wrapped) #641

Closed sydb closed 8 months ago

sydb commented 8 months ago
ebeshero commented 8 months ago

Summarizing how things went by the end of the day on Saturday 10/29 (EST). @sydb notes (from Slack):

Every test in Test/ now passes except test-epub, for which 13 of the output files get the error “This file should declare in the OPF the property: scripted”.

In order to get the XSLT to run, the top of tei-to-epub3.xsl looks like:

  <xsl:import href="../html/html.xsl"/>
  <xsl:import href="../odds/teiodds.xsl"/>   <!-- only included for $autoGlobal param, which ../odds/classatts.xsl relies on -->
  <xsl:import href="../odds/classatts.xsl"/> <!-- only included for ATTCLASSES key -->
  <xsl:import href="../epub/epub-common.xsl"/>
  <xsl:import href="../epub/epub-preflight.xsl"/>
  <xsl:import href="../html/html_oddprocessing.xsl"/> <!-- only included for schemaOut template, which ../odds/teiodds.xsl seems to need -->

Syd notes: We need to figure out the correct file for the "schemaOut" template (there are 5 of them, all different). All that work (i.e., importing 3 files which have a total of 4,390 elements and attributes) is being done just so we can execute this line in common_tagdocs.xsl : <xsl:variable name="thisClassSpec" select="key('ATTCLASSES', @key)" as="element(tei:classSpec)?"/>

Perhaps this is better than using key():

<xsl:variable name="key" select="@key"/>
<!-- note: following line does not use "key('ATTCLASSES', @key)" because ATTCLASSES is not defined in this or any imported file, and importing it requires importing other files, too. -->
<xsl:variable name="thisClassSpec" select="//tei:classSpec[ @ident eq $key]" as="element(tei:classSpec)?"/>
joeytakeda commented 8 months ago

I've now tested this (adding the all of those <xsl:import>s as described above) and get the same validation problem with the epub:

Validating against EPUB version 3.0
ERROR: actual-results/test_3.epub/OPS/index.html: This file should declare in the OPF the property: scripted
ERROR: actual-results/test_3.epub/OPS/index-front.1_div.1.html: This file should declare in the OPF the property: scripted
ERROR: actual-results/test_3.epub/OPS/index-front.1_div.2.html: This file should declare in the OPF the property: scripted
[... plus many more ....]

I believe this is caused by an import, which is adding all of the HTML javascript into the epubs but shouldn't be there — or, at least, isn't there in test_3.epub from the last Stylesheets release's actual-results folder .

Adding the following to the bottom of epub3/tei-to-epub3.xsl allows make test-epub to succeed:

  <!--Add empty javascript hook to remove JS-->
  <xsl:template name="javascriptHook"/>

The suggestion of replacing key() with a variable (and not adding a bunch of imports) is probably the right call; however, when I did that, different parts of Test failed, so some more investigation would be needed there to see if there are different and unexpected repercussions (sigh)

sydb commented 8 months ago

Implemented change I suggested above and which @joeytakeda thought was the way to go in 7ed5111f. Has passed test of building the Guidelines. Running Stylesheet tests now.

sydb commented 8 months ago

After removing <xsl:import href="../odds/classatts.xsl"/> from epub3/tei-to-epub3.xsl (which I forgot to do in previous commit, now done in 05231d48), this branch passes Test2/ but fails Test/ with a diff error (albeit one we may eventually to decide is OK).[1]

In Test/ the file actual-results/test15.odd.html has 4 extraneous empty <li class="classSpecItem" itemprop="item"> elements. Should we be worrying about that?

Note [1] This is a bit troublesome because it means Test2/ is missing something. But that is not the subject of this PR.

joeytakeda commented 8 months ago

In Test/ the file actual-results/test15.odd.html has 4 extraneous empty <li class="classSpecItem" itemprop="item"> elements. Should we be worrying about that?

@sydb: Do we know what is it that's causing them? At the moment, it produces an output that has (unsurprisingly) empty additional list items (below screenshot of output of actual-results/test15.odd.html), so I think we do need to worry about that:

Screenshot 2023-11-06 at 12 41 57 PM

I wonder if we should reverse course and take the brute-force approach of adding the imports and the empty javascript hook? While this is certainly more overhead, it does allow both test suites to pass (and I don't imagine would cause problems in other transformations, unlike changes to common)

sydb commented 8 months ago

No, I do not really know what is causing them.

As for going the “adding the imports and the empty javascript hook” route, do we know what this test file looks like with that system?

sydb commented 8 months ago

I found the bug, I think: the //classSpec that I used instead of the ATTCLASSES key needed to look at only <classSpec>s with an @type of "atts". We added the needed predicate. @martindholmes is pushing up the results now.