search

daisy / pipeline-scripts

!! NOTE: This project is now part of the pipeline-modules project !! | Script modules for the default DAISY Pipeline 2 distribution.
GNU Lesser General Public License v3.0
6 stars 5 forks source link

daisy202-to-epub3: whenever a epub:textref attribute is added to a SMIL, a "attribute-value" attribute with the same value is added #75

Closed josteinaj closed 7 years ago

josteinaj commented 9 years ago

For instance:

<seq xmlns:epub="http://www.idpf.org/2007/ops" id="mo1_seq278" epub:textref="content.html#dtb824" attribute-value="content.xhtml#dtb824">

attribute-value is an option on p:add-attribute so there's probably a bug with how that step is used to add the epub:textref.

bertfrees commented 7 years ago

@josteinaj I'm having a hard time figuring out from the code (mediaoverlay-utils + daisy202-to-epub3) when a epub:textref would be added to a SMIL. Can you give me a hint?

josteinaj commented 7 years ago

according to find+grep, these are the files where a textref is added:

I think the problem might be in ./modules/scripts/daisy202-to-epub3/src/main/resources/xml/convert/media-overlay.xpl though. I'm guessing this:

<xsl:attribute name="attribute-value" select="replace(@epub:textref,'^(.+)\.[^\.]*#(.*)$','$1.xhtml#$2')"/>

should've been this:

<xsl:attribute name="epub:textref" select="replace(@epub:textref,'^(.+)\.[^\.]*#(.*)$','$1.xhtml#$2')"/>
bertfrees commented 7 years ago

Yes that's probably it. But I hadn't started looking for the bug yet. I was still trying to find a test case that demonstrates the bug. I don't know what my input has to be in order to get a epub:textref in the output.

josteinaj commented 7 years ago

modules/scripts-utils/mediaoverlay-utils/src/main/resources/xml/conditionally-join-toplevel-seq-with-body.xsl seems to have an error as well, I'm guessing that's the problem:

<xsl:if test="mo:body/mo:seq/@epub:type">
    <xsl:attribute name="epub:textref" select="mo:body/mo:seq/@epub:type"/>
</xsl:if>

epub:textref here should've been epub:type.

bertfrees commented 7 years ago

OK I found out that in resolve-relative-uris.xsl, @epub:textref attributes are created from @src attributes on seq, and in rearrange.xsl, seq elements with @src are created for elements in a content document with @id that don't have a corresponding smil:text element but do have children with a corresponding smil:text element.

This way I could reproduce the bug.

bertfrees commented 7 years ago

Fixed by https://github.com/daisy/pipeline-scripts/pull/104

josteinaj commented 6 years ago

We're getting this error in daisy202-to-epub3 now.

The latest released version of the script is 1.2.1, which was released in ff94d64800f641c82553fb84138320e74fd27120.

➜  pipeline-scripts git:(master) git show master | head -n 1
commit fc36ae398dd0d620d267b760c54a359e8065d345
➜  pipeline-scripts git:(master) git log --oneline ff94d64800f641c82553fb84138320e74fd27120..HEAD daisy202-to-epub3 | cat
f7a2abe daisy202-to-epub3: fix issues #75 and #76
c3d96c7 daisy202-to-epub3: add a test for issues #75 and #76
0e1bab7 Fix issue #74
83012c8 Add a test for issue #74: smil references inside links should be removed
3207018 Fix daisy202-to-epub3 tests and run with modules-test-helper
46d8a2f [maven-release-plugin] prepare for next development iteration

So it seems that the patches for this issue haven't been released yet?

bertfrees commented 6 years ago

Yes, Romain just found out that I didn't release the scripts due to a stupid mistake. I'm planning to make a new release soon.

josteinaj commented 6 years ago

Ok. I'll try switching temporarily to the SNAPSHOT version for now.

bertfrees commented 6 years ago

If I have staged the release would you be willing test it?

josteinaj commented 6 years ago

Yes, sure.

josteinaj commented 6 years ago

I just noticed that the same error also occurs with epub3-to-daisy202 v1.1.1, so a new release of that script is also necessary.

bertfrees commented 6 years ago

@josteinaj I have staged the release: https://github.com/daisy/pipeline-assembly/pull/146.

josteinaj commented 6 years ago

  1. Downloaded linux version of v1.10.2 from DP2 homepage, and upgraded to v1.10.3 using updater.

  2. Tried reproducing issues

  1. Upgraded to the staged v1.10.4 as follows:
  1. Tried reproducing issues

So the staged v1.10.4 release fixes daisy202-to-epub3 but not epub3-to-daisy202.

bertfrees commented 6 years ago

@josteinaj Thanks! Maybe next time it's better to update to the staged release via the updater as well. You would need to set the descriptor URL to https://raw.githubusercontent.com/daisy/pipeline-assembly/rd-1.10.4/. I think that would work.

The #116 bug, that's probably an older one, right? Not a regression in 1.10.4?

josteinaj commented 6 years ago

Yeah I was wondering if I could use the updater but wasn't sure how. Will try that next time.

116 is not a regression no. It must just be the case that nobody have been using that script until now. Maybe it was introduced in a previous release, I don't know.