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

ERROR value of "severity" attribute in XML output conflicts with WARNING message #789

Closed
bitsgalore opened this issue Aug 29, 2017 · 11 comments
Labels
status: has PR The issue is being processed in a pull request type: improvement The issue suggests an improvement of an existing feature
Milestone

Comments

@bitsgalore
Copy link

Looking at the XML output of Epubcheck 4.0.2, sometimes the value of the "severity" attribute in a message element contradicts the actual warning/error message. To illustrate this see below EPUB:

epub20_crazy_columns.epub

I run this through Epubcheck using:

java -jar epubcheck.jar epub20_crazy_columns.epub -out epub20_crazy_columns.xml

The output contains a validation message about about the use of absolute positions:

<message severity="error" subMessage="CSS-017">CSS-017, WARN, [CSS selector specifies absolute position.], OEBPS/Styles/styles.css (13-1)</message>

It is strange that the severity attribute states that this is an error, whereas the text message says it is a warning. Seeing that this file passed validation I'm assuming that internally Epubcheck considers this to be a warning.

Another example is this file:

epub20_encryption_binary_content.epub

Here Epubcheck reports:

<message severity="error" subMessage="HTM-023">HTM-023, WARN, [An invalid XHTML Named Entity was found: '&amp;0;'.], OEBPS/Text/pdfMigration.html (18-197)</message>

Which again shows the same problem.

@bitsgalore bitsgalore changed the title Value of "severity" attribute in XML output conflicts with WARNING message ERROR value of "severity" attribute in XML output conflicts with WARNING message Aug 29, 2017
@tofi86
Copy link
Collaborator

tofi86 commented Aug 29, 2017

This clearly looks like a bug. Thanks for reporting it, we'll take a closer look!

@tledoux you introduced this in PR #673 for issue #486 - can you probably have a look at this? Thanks!

@tofi86 tofi86 added type: bug The issue describes a bug status: needs review Needs to be reviewed by a team member before further processing labels Aug 29, 2017
@tofi86 tofi86 added this to the 4.1.0 milestone Aug 29, 2017
@tledoux
Copy link
Contributor

tledoux commented Aug 29, 2017

Indeed, the main issue is that the schema jhove.xsd allows only two values for the severity attribute

<xs:attribute name="severity" use="optional">
  <xs:simpleType
    <xs:restriction base="xs:string">
      <xs:enumeration value="error"/>
      <xs:enumeration value="info"/>
    </xs:restriction>
  </xs:simpleType>
</xs:attribute>

So I had to decide how to map from the levels of epubcheck to these two values.
Currently the mapping is:

EpubCheck level severity attribute
fatal error error
error error
warn error
hint info

Maybe I should have made another choice? Should I lower the warn to info?

@bitsgalore
Copy link
Author

bitsgalore commented Aug 29, 2017

Hmm, tricky one. I'm not in favour of mapping the Epubcheck Warning level to jhove's "error" level, b/c epubcheck being a validator most users will expect this to be something that causes the validation to fail (which it doesn't).

Another option would be to add a "warning" level to the JHOVE schema and see if that gets accepted by the JHOVE maintainers (OPF). That change wouldn't cause any existing (valid) JHOVE output files to fail validating against its schema, and I think this could be useful for JHOVE as well. Not sure if they'd be open to this but perhaps worth a try?

@tofi86
Copy link
Collaborator

tofi86 commented Aug 29, 2017

Ah, well, I remember the discussion about that, @tledoux.

Personally I'm not very happy with the JHOVE schema anyway and would suggest to implement a EpubCheck prorietary schema once in a while. But for now we should stick to the JHOVE and do our best.

Should we remove the severity attribute entirely? Would be my best guess...

@tofi86 tofi86 removed the status: needs review Needs to be reviewed by a team member before further processing label Aug 29, 2017
@tofi86
Copy link
Collaborator

tofi86 commented Sep 26, 2017

Any comments on that topic? Shall we remove the severity attribute or leave it as it currently is?

@tofi86 tofi86 added the status: waiting for feedback The development team needs feedback from the issue’s creator label Sep 28, 2017
@tofi86
Copy link
Collaborator

tofi86 commented Nov 27, 2017

Any suggestions?

I'm voting for removing the severity attribute. Shall we do that with EpubCheck 4.1?

@rdeltour
Copy link
Member

I'd be in favor of keeping it as-is for 4.1, and maybe document this particularity (which can be confusing I agree).

In a next version however, we should revamp the XML format entirely, and we could possibly use a custom XML flavor (based on EARL?) on which we would have better control. People can always contribute XSLTs to convert to their flavor of choice.

@tofi86
Copy link
Collaborator

tofi86 commented Nov 29, 2017

Thanks @tledoux for pushing a fix in PR #814 and using severity info in the XML format for warning messages from EpubCheck.

I'm okay whith that for the moment, but will open another issue to not forget to implement another XML structure in a future release.

@tofi86
Copy link
Collaborator

tofi86 commented Nov 29, 2017

See issue #816 for further discussing a new XML output schema.

@tofi86 tofi86 added status: has PR The issue is being processed in a pull request and removed status: waiting for feedback The development team needs feedback from the issue’s creator labels Nov 29, 2017
@rdeltour
Copy link
Member

Since this is a breaking change, and most publishing workflow reject EPUBs with warnings, and we didn't receive that much feedback about this, I will leave this issue open and remove it from the v4.1 milestone.

Thank you @tledoux in any case for the PR, let's keep it open as well until we decide how to further proceed!

@rdeltour rdeltour removed this from the 4.1.0 milestone Nov 20, 2018
@rdeltour rdeltour added type: improvement The issue suggests an improvement of an existing feature and removed type: bug The issue describes a bug labels Jan 21, 2019
tledoux added a commit to tledoux/epubcheck that referenced this issue Aug 10, 2019
In order to fix the issue w3c#789, the new jhove schema is implemented.

Indeed, this new schema has been published and includes an 'id' attribute as well as a 'warning' level. So the alignment between the severity attribute and the level is now straightforward.

EpubCheck level | severity attribute
----------------| ------------------
fatal error     | error
error           | error
warn            | warning
hint            | info
tledoux added a commit to tledoux/epubcheck that referenced this issue Aug 10, 2019
In order to fix the issue w3c#789, the new jhove schema is implemented.

Indeed, this new schema has been published and includes an 'id' attribute as well as a 'warning' level. So the alignment between the severity attribute and the level is now straightforward.

EpubCheck level | severity attribute
----------------| ------------------
fatal error     | error
error           | error
warn            | warning
hint            | info
@tledoux
Copy link
Contributor

tledoux commented Aug 10, 2019

As described in the PR, the new jhove schema allows for a 'warning' level which allows to map directly epubcheck levels with severity.
It also has an 'id' attribute to handle the id of an error.

I hope this PR could really fix this issue.

@rdeltour rdeltour added this to the 4.2.3 milestone May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: has PR The issue is being processed in a pull request type: improvement The issue suggests an improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants