Project

Profile

Help

Bug #5081

closed

Result of serialize($someElement, map{}) contains XML declaration although function spec suggest default value is to omit-xml-declaration

Added by Martin Honnen over 2 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Low
Assignee:
-
Category:
XPath Conformance
Sprint/Milestone:
Start date:
2021-09-06
Due date:
% Done:

100%

Estimated time:
Applies to JS Branch:
2
Fix Committed on JS Branch:
2
Fixed in JS Release:
SEF Generated with:
Platforms:
Company:
-
Contact person:
-
Additional contact persons:
-

Description

The fn:seriaiize spec at https://www.w3.org/TR/xpath-functions/#func-serialize defines that "If the second argument is supplied as a map ... The required type of each parameter, and its default value, are defined by the following table" saying

omit-xml-declaration	xs:boolean?	true() means "yes", false() means "no"	yes

For some reason, however, Saxon-JS 2.3 on both the browser side as well as in Node.js outputs an XML declaration e.g.

<?xml version="1.0" encoding="utf-8"?>
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
  version="3.0"
  xmlns:xs="http://www.w3.org/2001/XMLSchema"
  exclude-result-prefixes="#all"
  expand-text="yes">
  
  <xsl:output method="html" html-version="5.0" indent="yes"/>

  <xsl:template match="/" name="xsl:initial-template">
    <xsl:variable name="foo" as="element(foo)"><foo>test</foo></xsl:variable>
    <xsl:copy-of select="$foo"/>
    <section>
      <h2>serialize with empty map</h2>
      <pre>{serialize($foo, map{})}</pre>
    </section>
    <xsl:comment>Run with {system-property('xsl:product-name')} {system-property('xsl:product-version')} {system-property('Q{http://saxon.sf.net/}platform')}</xsl:comment>
  </xsl:template>
  
</xsl:stylesheet>

outputs (for Saxon JS 2.3 browser)

<foo>test</foo>
<section>
   <h2>serialize with empty map</h2>
   <pre>&lt;?xml version="1.0" encoding="UTF-8"?&gt;&lt;foo&gt;test&lt;/foo&gt;</pre>
</section>
<!--Run with Saxon-JS 2.3 Browser-->
Actions #1

Updated by Norm Tovey-Walsh over 2 years ago

  • Status changed from New to Resolved

Indeed, the code that outputs an XML declaration was testing for true explicitly instead of trueOrAbsent. Fixed.

Actions #2

Updated by Norm Tovey-Walsh over 2 years ago

  • Status changed from Resolved to In Progress

In code review, Debbie observed that my fix is in the serializer rather than in the fn:serialize() function. It's possible (likely?) that this has a broader impact than intended.

Curiously, there appear to be no QT3 tests for fn:serialize() that test the default behavior of omit-xml-declaration when the options are provided as a map.

Action: add some more tests and make sure we've put the fix in the right place.

Actions #3

Updated by Michael Kay over 2 years ago

Generally, coverage of serialization options in QT3 is very patchy indeed.

Actions #4

Updated by Debbie Lockett over 2 years ago

As predicted, the "fix" for this bug is causing regression in the XSLT3 test suite tests: in the output and result-document test sets. (Note that the XSLT 3.0 specification says that the default for xsl:output/@omit-xml-declaration is no.)

I have committed changes in Serializer.js to reverse the "fix" previously made.

Still TODO:

  • Fix this specific bug in the implementation of fn:serialize (in CoreFn.js). (In fact, inspecting the code, I have spotted other issues relating to defaults in fn:serialize; to be raised in another bug issue.)
  • Decide what the default for omit-xml-declaration should be for the SaxonJS API method SaxonJS.serialize. Previously the default was false() (aligning with the default for xsl:output/@omit-xml-declaration in the XSLT 3.0 spec), and the tests worked with this (however the documentation lacks information). Along with the code "fix" changes made for this bug, lots of the unit tests were changed. Should these changes also be reversed? Or do we want the default for SaxonJS.serialize to align with that for fn:serialize?
Actions #5

Updated by Debbie Lockett over 2 years ago

QT3 test serialize-xml-127a added in fn-serialize test set.

Actions #6

Updated by Debbie Lockett over 2 years ago

  • Status changed from In Progress to Resolved
  • Fix Committed on JS Branch 2 added

Changes to various unit tests committed, to reverse the previous changes. (The default for omit-xml-declaration in the underlying Serializer remains as false; in particular this remains the default for SaxonJS.serialize too.)

Bug fix committed in CoreFn.js, to add the omit-xml-declaration=true default for fn:serialize when called with a map as the 2nd argument.

Actions #7

Updated by Debbie Lockett almost 2 years ago

  • Status changed from Resolved to Closed
  • % Done changed from 0 to 100
  • Fixed in JS Release set to SaxonJS 2.4

Bug fix applied in the SaxonJS 2.4 maintenance release.

Please register to edit this issue

Also available in: Atom PDF Tracking page