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

Unreachable XPath in Jing's built-in Schematron XSLT causes a Saxon warning #240

Closed
rdeltour opened this issue Dec 6, 2018 · 11 comments
Closed

Comments

@rdeltour
Copy link

rdeltour commented Dec 6, 2018

The built-in Schematron XSLT contains the XPath expression /.. which will always evaluate to the empty sequence. This causes a static warning to be issues by Saxon 9 when compiling a Schematron schema:

Warning at char 3 in xsl:param/@select on line 459 column 43 
  SXWN9000: The parent axis starting at a document node will never select anything

Maybe Jing's built-in Schematron XSLT could be updated to more recent versions from schematron/schematron?

Ping @sideshowbarker @tgraham-antenna @georgebina

@rdeltour
Copy link
Author

rdeltour commented Dec 6, 2018

A repro is available in this test project. See also w3c/epubcheck#859.

Note that this can be worked around by configuring a transformer factory with static error listener, as we did in EPUBCheck.

@sideshowbarker
Copy link
Contributor

Maybe Jing's built-in Schematron XSLT could be updated to more recent versions from schematron/schematron?

Could you be more specific about what such an update would involve? It doesn’t seem like the file could simply be replaced, at least — because it has elements in the http://www.thaiopensource.com/ns/error, etc., namespaces, along with corresponding code in https://github.com/relaxng/jing-trang/blob/V20181204/mod/schematron/src/main/com/thaiopensource/validate/schematron/SchemaReaderImpl.java which relies on those elements.

@rdeltour
Copy link
Author

rdeltour commented Dec 6, 2018

Could you be more specific about what such an update would involve?

To be honest I don't now specifically. I just notice that Jing's schematron XSLT is quite old and as far as I understand Schematron's "skeleton" XSLTs have been updated since then; but I don't know what it entails exactly for Jing's integration. That's why I cc'd @tgraham-antenna (who if I'm not mistaken is maintaining the Schematron project) and @georgebina (who –again if I'm not mistaken– worked in the past in ISO Schematron implementation in Jing).

@ndw
Copy link
Contributor

ndw commented Dec 10, 2018

I can confirm that mod/schematron/src/main/com/thaiopensource/validate/schematron/resources is pretty old.

An additional complication is that (I believe) Schematron now has XSLT 1.0 and XSLT 2.0 code paths. If Jing intends to support both 1.0 and 2.0 processors, I think it'll have to be updated to do a little bit of dynamic configuration.

Here's hoping either @tgraham-antenna or @georgebina have already worked through these issues.

@tgraham-antenna
Copy link
Contributor

The built-in Schematron XSLT contains the XPath expression /.. which will always evaluate to the empty sequence. This causes a static warning to be issues by Saxon 9 when compiling a Schematron schema:

/.. is/was an XSLT-1.0ism for selecting an empty node-set. Sequences don't exist in XSLT 1.0/XPath 1.0, so using () for selecting the empty sequence wasn't an option. (You might hope that the Saxon folks could have been charitable and issued a special warning for this usage in an XSLT 1.0 stylesheet, but they're not in the business of supporting XSLT 1.0.)

Schematron does need a purely XSLT 2.0 code base (Schematron/schematron#55) (as well as a purely XSLT 3.0 one). I have been trying to do things properly (for some value of 'properly') by getting a functioning, easily extendable test suite before duplicating the code, but the available time hasn't matched my ambitions.

@sideshowbarker
Copy link
Contributor

/.. is/was an XSLT-1.0ism for selecting an empty node-set. Sequences don't exist in XSLT 1.0/XPath 1.0, so using () for selecting the empty sequence wasn't an option. (You might hope that the Saxon folks could have been charitable and issued a special warning for this usage in an XSLT 1.0 stylesheet, but they're not in the business of supporting XSLT 1.0.)

Given that, I’ve gone ahead and made a patch in #241 that suppresses the warning.

Note that this can be worked around by configuring a transformer factory with static error listener, as we did in EPUBCheck.

The patch in #241 is essentially just an upstreaming of that same workaround.

@sideshowbarker
Copy link
Contributor

sideshowbarker commented Dec 13, 2018

If anybody following this issue objects to the workaround in #241, please let me know. Otherwise, tomorrow I’ll go ahead and merge the patch and publish a new release (both a new Maven package and also a release here).

@georgebina
Copy link
Contributor

georgebina commented Dec 13, 2018

Looking at that stylesheet the check-cycles mode is called always on a rule providing also the nodes parameter, so basically there is no problem if we just remove the default "/.." value. That should be a simpler fix I guess, just delete select="/.." from the xsl:param element

.

@sideshowbarker
Copy link
Contributor

Looking at that stylesheet the check-cycles mode is called always on a rule providing also the nodes parameter, so basically there is no problem if we just remove the default "/.." value. That should be a simpler fix I guess, just delete select="/.." from the xsl:param element

Well, that’s good news — thanks! So I’ve opened #244 with a patch which does that.

@dmj
Copy link

dmj commented Dec 14, 2018

Just a heads up. I currently struggle with SXWN9000 in schxslt/schxslt#11 -- This static error is also triggered when a Schematron rule matches e.g. a text() or comment(), i.e. Saxon is used to execute a compiled validation stylesheet.

As far as I can tell there is no way to prevent this in XSLT ☹️

@sideshowbarker
Copy link
Contributor

sideshowbarker commented Dec 15, 2018

Just a heads up. I currently struggle with SXWN9000 in dmj/schxslt#11 -- This static error is also triggered when a Schematron rule matches e.g. a text() or comment(), i.e. Saxon is used to execute a compiled validation stylesheet.

Yeah, a web search for "SXWN9000" seems to indicate there are a number of cases under which that warning gets emitted now, under circumstances where developers are trying to figure out how to suppress the warning or work around it.

And @rdeltour notes at w3c/epubcheck#859 (comment)

Apparently Saxon 9.8 raises a SXWN9000 warning for this construct, when Saxon 9.7 stayed silent. I ran the tests before w3c/epubcheck#830 was applied and the warning didn't show up.

So while this change in Saxon 9.8 might seem like a regression from the point of view of anybody who’d been using Saxon 9.7 or before, it’s formally/technically correct — and I can say that in my work on the HTML checker, I’ve also introduced new warnings that have the side effect of potentially breaking people’s existing builds and such. But I’ve only done it when it seems the change it likely to be more helpful overall as far as giving developers more assistance in finding problems they might have otherwise missed. So I trust that’s the same philosophy that motivated the Saxon 9.8 change for this.

All that said, it’s always frustrating to be one of the N downstream developers/consumers who get stuck dealing with the effects of that kind of change…

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

No branches or pull requests

6 participants