Bug #4783
closedIncorrect results when comment node is created as child of document node
100%
Description
Test case function-lookup-008 is failing. If the test is modified by removing the final xsl:comment
instruction (which is there only to add diagnostics) then the test passes. Somehow the addition of the final document-level comment node is causing the constructed tree to be incorrect.
Updated by Michael Kay about 4 years ago
The raw result of the transformation consists of a parentless element node (which at the implementation level actually belongs to a DocumentFragment), followed by a parentless Comment node.
It's then up to the fn:transform() function to turn this sequence into a document tree by wrapping in a document node. The code to do this is at CoreFn#2776, and it appears to ignore everything in the sequence except element nodes and text nodes.
For some reason a call to Copy.makeComplexContent() has been commented out here.
I think the correct logic is to create a Builder, issue a startDoc(), append() each item in the raw result sequence, then issue endDoc().
Updated by Michael Kay about 4 years ago
I've considerably simplified the logic in fn:transform() so it now does:
if (buildTree) {
Diag.assert(Array.isArray(r));
const builder = getBuilder(context);
const out = Push.getComplexContentOutputter(builder);
out.startDoc();
r.forEach(it => out.append(it));
out.endDoc();
out.close();
r = builder.sequence;
}
and this test now passes. But it's a big change, so it needs careful regression testing.
Updated by Michael Kay about 4 years ago
In the XSLT3 test suite, the change has fixed a few outstanding test failures (e.g. in accumulator), but has also resulted in a handful of new test failures, notably in current-output-uri, message, mode, output, position, result-document, try. I will investigate these.
In QT3 we're seeing 13 failures in fn-transform and one failure in xs-numeric (which is probably induced by previous bug fixes since I last ran QT3).
Updated by Michael Kay about 4 years ago
Homing in on the fn:transform failures, they are failures in the line
Diag.assert(Array.isArray(r));
included in the new code above.
The existing code tries to cope with r (the raw result) being either an array or a singleton. I'd prefer to be consistent about it always being an array if possible, which is why I added the assertion.
Updated by Michael Kay about 4 years ago
In fn-transform-14 there is no principal result document (only secondary results), and r
is null. Bypassing the tree construction if r===null I now get 11 failures in fn:transform, for example fn-transform-35.
In fn-transform-35, the request is for delivery-format:"serialized": it seems that the serialized form has been wrapped in a new doccument node and serialized again. The problem seems to be that the buildTree variable is true even though what we have is already a serialized result.
CoreFn#2694 sets let buildTree = destinationType !== "raw";
. Changing that so we don't set buildTree if the destination is "serialized" means we now pass all the fn:transform tests.
Updated by Michael Kay about 4 years ago
- Status changed from New to Resolved
- Applies to JS Branch 2.0, Trunk added
- Fix Committed on JS Branch 2.0, Trunk added
This change fixes the remaining regressions in the QT3 and XSLT3 tests; marking as resolved.
Updated by Community Admin almost 4 years ago
- Applies to JS Branch 2 added
- Applies to JS Branch deleted (
2.0, Trunk)
Updated by Community Admin almost 4 years ago
- Fix Committed on JS Branch 2 added
- Fix Committed on JS Branch deleted (
2.0, Trunk)
Updated by Debbie Lockett over 3 years ago
- Status changed from Resolved to Closed
- % Done changed from 0 to 100
- Fixed in JS Release set to Saxon-JS 2.1
Bug fix applied in the Saxon-JS 2.1 maintenance release.
Please register to edit this issue
Also available in: Atom PDF Tracking page