Project

Profile

Help

Bug #3956

closed

Incorrect base URI for element nodes within a secondary result document

Added by Michael Kay about 6 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
XSLT conformance
Sprint/Milestone:
-
Start date:
2018-10-09
Due date:
% Done:

100%

Estimated time:
Legacy ID:
Applies to branch:
9.8, 9.9
Fix Committed on Branch:
9.9
Fixed in Maintenance Release:
Platforms:

Description

Within <xsl:result-document>, element nodes in the output tree should have the same base URI as the URI of the result document itself, by virtue of rule 11 of 5.7.1 Constructing Complex Content. It is reported (by Norm Walsh on the saxon-help mailing list) that this is not the case in 9.9, the base URI of such element nodes is the static base URI of the stylesheet.

Actions #1

Updated by Michael Kay about 6 years ago

  • Status changed from New to In Progress

Added XSLT3 test case result-document-0102 which demonstrates the problem.

Actions #2

Updated by Michael Kay about 6 years ago

Note that Saxon 9.8 is also failing this test.

There's this horrible code in TinyBuilder#272

        if (!pipe.isLocationIsCodeLocation() && location.getSystemId() != null) {
            tt.setSystemId(nodeNr, location.getSystemId());
        } else if (currentDepth == 1) {
            tt.setSystemId(nodeNr, systemId);
        }

The intent here is to take the system ID of a new element node either from the Location passed with the startElement event, or from the systemId property of the TinyBuilder, depending on a property of the PipelineConfiguration. On this occasion we're taking the value from the Location parameter, which is the stylesheet base URI.

This is the only place that pipe.isLocationIsCodeLocation() is called. The underlying property is false by default, and is set to true on most node creation paths (e.g. ElementCreator.constructElement) - but not on push-mode paths such as ElementCreator.processLeavingTail().

Actions #3

Updated by Michael Kay about 6 years ago

The obvious thing is for ResultDocument to call pipe.setLocationIsCodeLocation(true) and this indeed causes the test to pass.

However, it's deeply flawed, because (as far as I can see) all ResultDocument threads are sharing the same PipelineConfiguration and therefore modifying its properties in situ is going to cause trouble.

In this particular scenario, the result document is being created using an XdmDestination, and the base URI of the destination has been set using XdmDestination.setBaseURI(). The builder is created using XdmDestination.getReceiver(), and this sets the base URI property on the builder. I feel that in this situation, the base URI on the builder should take precedence over anything in the Location property of the startDocument event.

When we're building in push mode, there seem to be essentially two cases to consider:

(a) building from events originating with an XML parser. In this case the system ID of a node comes from the location information, which can change from one node to another based on the system ID of external entities (or XInclude, though that's slightly different). The base URI of the node in this case is a function of the system ID and any xml:base attributes.

(b) building from instructions in a query or stylesheet. In this case the base URI of the root node comes from the static base URI of the relevant instruction in the query/stylesheet, and the base URIs of other nodes in the tree are the same as the root, subject to any xml:base attributes.

The "locationIsCodeLocation" flag in the pipeline configuration is intended to distinguish these two cases.

Because of multithreading, etc, it would seem safer to have this flag in the builder itself. Surely we always know when creating the builder which situation we are in?

I've tried creating this flag in the builder and setting it in appropriate places. 3 failures in the XSLT 3 test suite: base-uri-024, -037, -041.

With some tweaks to the places where we set the new flag, all tests now seem to be working. Need to run more of the unit tests, however.

Actions #4

Updated by Michael Kay about 6 years ago

  • Status changed from In Progress to Resolved
  • Priority changed from Low to Normal
  • Applies to branch 9.8, 9.9 added
  • Fix Committed on Branch 9.8, 9.9 added

Also added two new tests for base-uri of nodes read using doc/document functions, after spotting that a newly introduced bug in this area didn't cause any tests to fail.

I'm committing this change for 9.9. In summary:

  • Drop the existing PipelineConfiguration.isLocationIsCodeLocation flag

  • Add a flag on Builder: useEventLocation, defaulting to true.

  • Set useEventLocation to false on various paths, typically where the stylesheet/query is constructing a temporary tree

  • In TinyBuilder.startElement(), set the systemId from the location parameter if useEventLocation is set, otherwise use the systemId property of the builder.

(Note all XSLT3 tests are working with -tree:ilnked. I'm not sure why, but it ain't broke so I'm not taking it apart to find out how it works)

On 9.8 I'm committing a much more modest change, setting the locationIsCodeLocation flag on the ResultDocument path (it's already set, but the code ends up using a different PipelineConfiguration).

Actions #5

Updated by O'Neil Delpratt almost 6 years ago

  • % Done changed from 0 to 100
  • Fixed in Maintenance Release 9.8.0.15 added

Bug fix applied in the Saxon 9.8.0.15 maintenance release. Leave open to the Saxon 9.9 maintenance release.

Actions #6

Updated by O'Neil Delpratt almost 6 years ago

  • Status changed from Resolved to Closed
  • Fixed in Maintenance Release 9.9.0.2 added

Bug fix applied in the Saxon 9.9.0.2 maintenance release.

Actions #7

Updated by O'Neil Delpratt almost 6 years ago

  • Status changed from Closed to In Progress
  • Fixed in Maintenance Release deleted (9.8.0.15, 9.9.0.2)

Gerrit Imsieke reported on the Saxon mailing list that the fix applied on the Saxon 9.8 and 9.9 branches have still not entirely addressed the bug.

I am reopening this bug issue as it has not completely resolved the problem.

Actions #8

Updated by O'Neil Delpratt almost 6 years ago

  • Fix Committed on Branch deleted (9.8)

Bug fixed in the TransformerReceiver to set the eventLocation property to false. For the next-in-chain we pipe the properties to the stylesheet builder, therefore it was missing the setting of the eventLocation.

Still investigating a fix for Saxon 9.8.

Actions #9

Updated by Michael Kay over 5 years ago

  • Status changed from In Progress to Resolved

I'm closing this on the basis that we are unlikely to do any further investigation of problems specific to 9.8 unless there is a specific bug report with a specific repro, and no practicable workaround.

Actions #10

Updated by O'Neil Delpratt over 5 years ago

  • Status changed from Resolved to Closed
  • Fixed in Maintenance Release 9.9.1.2 added

Bug issue fixed in the Saxon 9.9.1.2 maintenance release.

Please register to edit this issue

Also available in: Atom PDF