openSUSE / daps

DocBook Authoring and Publishing Suite (DAPS)
https://openSUSE.github.io/daps
Other
62 stars 19 forks source link

<imageobject ../> without role inside of <mediaobject .../> brings epub build down #382

Closed hustodemon closed 10 months ago

hustodemon commented 7 years ago

Problem description

Building SUSE Manager docs (https://github.com/SUSE/doc-susemanager/) using daps -d DC-create-all epub fail without a meaningful error message. Actual output:

runtime error: file /usr/share/daps/daps-xslt/common/copy.xsl line 31 element copy
Cannot add an attribute node to a non-element node.
runtime error: file /usr/share/daps/daps-xslt/common/copy.xsl line 31 element copy
Cannot add an attribute node to a non-element node.
error: file /doc-susemanager/build/.profiled/x86_64_zseries_power_kvm4x86_sles_mgr/MAIN-manager.xml
xsltRunStylesheet : run failed
/usr/share/daps/make/epub.mk:154: recipe for target '/doc-susemanager/build/.tmp/epub_create-all.xml' failed
make: *** [/doc-susemanager/build/.tmp/epub_create-all.xml] Error 11

I've discovered that this part of our document is the culprit:

<mediaobject>
    <imageobject role="fo">
        <imagedata fileref="exception-channels.png" width="444"/>
    </imageobject>
    <imageobject>
        <imagedata fileref="exception-channels.png" width="80%"/>
    </imageobject>
</mediaobject>    

In particular, the 2nd <imageobject> is missing the role attribute. Adding role="html" makes the build work.

I've taken a look at the daps xslt sources and it seems that reverting bdac7a1eaee036c616dbf7c834515ffd076aab2b also "fixes" the problem (although I believe there is a good reason for this patch ;) ).

Daps version: 2.3.0

fsundermeyer commented 7 years ago

toms, any comments? Looks like a bug to me....

tomschr commented 7 years ago

@hustodemon Thanks for your report! :+1:

Last week, Joseph and I sat together and we could confirm it. I thought I've fixed the above error message already ("element copy Cannot add an attribute node to a non-element node"), but it seems it isn't.

The strange thing is, I couldn't reproduce it on my system (openSUSE 42.1 with daps 2.3.0). The epub is built correctly...

tomschr commented 7 years ago

@hustodemon: could you check which DocBook stylesheets do you have?

$ rpm -q docbook-xsl-stylesheets docbook5-xsl-stylesheets

I have these:

docbook-xsl-stylesheets-1.78.1+svn9743-56.9.noarch
docbook5-xsl-stylesheets-1.78.1-10.6.noarch
hustodemon commented 7 years ago

@tomschr Yeah, I had the fear that this could be reproducible on my setup - there is a lot of variables in the equation :smile:

The packages I've got:

docbook-xsl-stylesheets-1.79.0-61.1.noarch # from obs://build.opensuse.org/Publishing
docbook5-xsl-stylesheets-1.78.1-2.4.noarch # from SLE-SDK-SP1

I'm on SLE12 SP1, getting this to work required some tuning. I needed to add some extra repos from the external build service:

http://download.opensuse.org/repositories/Publishing/SLE_12_SP1/
http://download.opensuse.org/repositories/Documentation:/Tools/SLE_12_SP1/
http://download.opensuse.org/repositories/M17N:/fonts/SLE_12/ # shazanami-fonts dependency :)

So there is also a possibility that the constellation of my environment is wrong (it's a docker container and I can provide the Dockerfile, if needed). In this case feel free to close the bug.

jcayouette commented 7 years ago

This feels like a feature that daps should support. Why should a writer need to enter two separate rolls for outputs? :P This currently must be done for every image in a document. A writer should be able to supply daps with an output format and daps should then adjust image formats automatically based on the specified output. Perhaps this code could be placed in each subfolder in stylesheets: epub, fo, etc. Would be a nice addition! :) I assume if your performing a packaging operation or creating online-docs you call the correct rules for each output that will be created.

tomschr commented 7 years ago

@jcayouette

Why should a writer need to enter two separate rolls for outputs? :P This currently must be done for every image in a document.

and

A writer should be able to supply daps with an output format and daps should then adjust image formats automatically based on the specified output.

Not sure if I fully understood your suggestion. However, I fear, DAPS cannot do format adjustments that you envisioned automatically. It's a little bit more. It's true, inserting images for different formats can be cumbersome.

Basically, there are two reasons why you have different <imageobject>s inside a <mediaobject>:

DAPS cannot know about these conditions. These conditions can't be determined automatically. That's the reason why you (as a writer) need to provide these.

On the other side: usually you don't need additional <imageobject>s if you don't need a different image width nor a different image format. In most cases you only need one:

<imageobject>
   <imagedata fileref="foo.png" width="80%"/>
</imageobject

This <imageobject> is used for all output formats. Of course, you will end up with the same width and image format. If you are happy with that, do it! DocBook (and GeekoDoc) doesn't force you to add more <imageobject>s if you don't want to. :smiley:

Does that answer your questions?

jcayouette commented 7 years ago

So if I understand correctly there is no way to create a stylesheet that can define a default layout for pdf or html that daps can call when you specify the format?

daps -d DC-quickstart-guide html would use an html stylesheet that sets the proper width for html images. daps -d DC-quickstart-guide pdf would use a pdf stylesheet that sets the proper width for pdf images.

Its 100% OK if its not possible, I just think it would be a cool feature! :) thanks for the reply Toms!

fsundermeyer commented 7 years ago

So if I understand correctly there is no way to create a stylesheet that can define a default layout for pdf or html that daps can call when you specify the format?

First, DAPS != (layout) stylesheets Yes, DAPS includes stylesheets, but these are solely used to transform the XML into XML, e.g. for profiling. The error hustodemon has reported is caused by one of these "xml2xml" stylesheets.

Second, and that is what you are referring to, DAPS does not (and cannot) care about layout. This is handeled by the "layout" stylesheets that you can pass to DAPS with --styleroot. Consider a single DocBook XML source from which you would like to create the following output formats:

Each output format needs to display images in different sizes. Since only the layout stylesheets "know" for which output format they are designed, image sizes need to be handled by these stylesheets. That said, width and height properties in DocBook XML have limited use and the author of a layout stylesheet may choose to completely ignore them (as is done in the suse2013 HTML sytlesheets).

To cut a long story short, YES, there is a way to define a stylesheet that can define a default layout for pdf or html - it's the layout stylesheet you call with --styleroot (or specify in the DC file).

As for the image formats (e.g. PNG vs. SVG) it would theoretically also be possible to ignore any given definitions from the and to force the use of e.g. PNG for HTML and SVG for PDF. However, that would require to entirely rely on DAPS automatically converting images. DAPS does this, but sometimes the outcome of the conversion may not be as good as you would like it to be. Therefore you have the option to explicitly specify the format in the tag.

This is especially required for all SVGs containing color gradients. FOP and XEP only support the SVG 1.0 specification which does not allow color gradients--builds with an SVG 1.1 specification cause the build to fail. DAPS automatically converts all SVGs to PDF and a tag like

<imageobject  fileref="foo.svg" format=PNG">

tells the layout stylesheets to use foo.png, although the original image is an SVG image. This, of course, needs to be supported by the layout stylesheets.

jcayouette commented 7 years ago

Ahhh ok, this makes sense! Thanks Frank for taking the time to explain this.

fsundermeyer commented 10 months ago

Invalid as per my comment above