Project

Profile

Help

Bug #5389

open

Test case attribute-1301 fails

Added by Michael Kay 5 months ago. Updated about 2 months ago.

Status:
In Progress
Priority:
Low
Assignee:
Category:
XSLT Conformance
Sprint/Milestone:
-
Start date:
2022-03-13
Due date:
% Done:

100%

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

Description

This test is doing something pretty weird with namespaces and gives the wrong results for SaxonJS, with either the XJ or XX compilers. I don't think it's a new problem, just one that we didn't consider important to fix. Adding it to the exceptions list for now.

Actions #1

Updated by Michael Kay 5 months ago

Let's look at the test. It's doing

<xsl:template match="/">
  <xsl:variable name="temp">
  <out>
    <jam>
      <xsl:attribute name="{docs/b}" namespace="http://xyz.com">jaminben</xsl:attribute>
    </jam>
  </out>
  </xsl:variable>
  <ans bdd="{namespace-uri-for-prefix('bdd', $temp/out/jam)}" attr-ns="{namespace-uri($temp/out/jam/@*:attr)}"/>
</xsl:template>

The source document is

<docs><b>bdd:attr</b></docs>

The expected output is:

<ans xmlns:bdd="http://bdd.test.com" bdd="http://bdd.test.com" attr-ns="http://xyz.com"/>

but it's failing:

Transformation failure: Error FORG0001 at attribute-1301.xsl#13
    Invalid local name: '' (prefix='ns0', uri='http://xyz.com')

Actions #2

Updated by Michael Kay 5 months ago

The rules for literal result elements say, firstly, that within the temporary tree $temp, the constructed jam element should have a namespace binding xmlns:bdd="http://bdd.test.com", copied from the xsl:stylesheet element.

The jam element should also have an attribute with local name attr, namespace URI http://xyz.com. The prefix for this attribute would normally be bdd. but this would create a conflict, so a different prefix must be allocated, and the appearance of ns0 in the error message suggests that it is attempting to allocate a prefix.

The error is coming from the XdmQName constructor (XdmAtomic.js line 754). This is being called from computedQNameEvaluator() (Push.js line 1123). The value of the variable name (line 1128) is a zero-length string.

Hmm.. I suspect that this error is genuine, docs/b isn't selecting anything, probably because we've selected the wrong source document. Try again.

Actions #3

Updated by Michael Kay 5 months ago

That's better. Now running with the correct source document, and getting output

<ans xmlns:bdd="http://bdd.test.com" bdd="http://xyz.com" attr-ns="http://xyz.com"/>

compared with the expected

<ans xmlns:bdd="http://bdd.test.com" bdd="http://bdd.test.com" attr-ns="http://xyz.com"/>

So the problem is that when we created the attribute, the prefix proposed in xsl:attribute/@name was used, despite the existence of a conflict with the binding of that prefix inherited by the literal result element.

Actions #4

Updated by Michael Kay 5 months ago

And looking at the code of computedQNameEvaluator(), at Push line 1165, we see

// TODO: handle conflicts where the prefix needs to be changed - see stylesheetResolver

But actually, that's probably not where it should be done. It's only when we get to the ComplexContentOutputter that we have enough information to know that there's a conflict.

The logic that attempts this is at Push.js line 177

                    const p2 = this.ensureBinding(name.prefix, name.uri, false);
                    if (p2 !== name.prefix) {
                        name = name.withPrefix(p2);
                    }

But when we call ensureBinding("bdd", "http://xyz.com", false), the map pendingNamespaces is empty, so no conflict is detected.

The problem seems to be that we are only looking in pendingNamespaces for an existing binding, we are not looking in namespaceBindings. Because the binding ("bdd", "http://bdd.test.com") is effectively inherited from the parent element (out), it's present in namespaceBindings but not in pendingNamespaces.

Actions #5

Updated by Michael Kay 5 months ago

Frustrating progress. With minor tweaks to this code I can get this test case working easily enough, but it always breaks something else.

Actions #6

Updated by Michael Kay 5 months ago

I seem to have found the right magic to make this test pass without breaking anything else. The code, however, feels fragile.

Actions #7

Updated by Michael Kay 5 months ago

  • Status changed from New to Resolved
  • Applies to JS Branch 2, Trunk added
  • Fix Committed on JS Branch 2, Trunk added
Actions #8

Updated by Debbie Lockett 3 months 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.

Actions #9

Updated by Debbie Lockett about 2 months ago

  • Status changed from Closed to In Progress
  • Fixed in JS Release deleted (SaxonJS 2.4)

Reopening this bug, because the test attribute-1301 is failing again. In fact, it looks like the bug was not fixed in the SaxonJS 2.4 release as previously stated, since the test results show the test was failing then too.

I have reinstated the test exception in the exceptions list.

Actions #10

Updated by Debbie Lockett about 2 months ago

Before finding this existing bug and the notes, I spent some time trying to debug the attribute-1301 test failure, but don't really feel like I've got anywhere. I don't feel like I've got a grasp on what's going on in the ComplexContentOutputter. I made some attempts to fix the code, but these only resulted in causing other test failures (much like Mike previously found, see #note-5).

Please register to edit this issue

Also available in: Atom PDF Tracking page