Saxon

This is a complicated area. The location passed to an Outputter is generally the location of the instruction that constructs the node, not the location of the node itself. The copy-of instruction takes special (somewhat devious) measures to ensure that the location of the node being copied is passed to the DocumentBuilder that constructs the copy of the node; in some cases (not all) the new node retains the location of the old node.

Also complicating this is that the location of a node isn't in all cases the same thing as its base URI.

Need more detail on this one before making any changes.

That's fair. Here's some context from a Slack chat:

Here's a weird one. I'm processing an XInclude. I construct a tree for the included content by calling various methods on the Receiver. Let's say the content I construct is . The baseURI (which comes from the underlying nodes getSystemId()) for foo is fine, for ?pi? is "", and for bar is "". The presence of the PI makes the following element(s) not have a base URI. I thought this was caused by my failure to supply a base URI for the PI. But I've changed the code so that I do this:

    public void addPI(XdmNode node) {
        Location location = VoidLocation.instance();
        if (node.getBaseURI() != null) {
            location = new SysIdLocation(node.getBaseURI().toString());
            receiver.setSystemId(node.getBaseURI().toString());
        }
        addPI(node.getNodeName().getLocalName(), node.getStringValue(), location);
    }

where the 3 argument form of addPI is:

    public void addPI(String target, String data, Location location) {
        try {
            receiver.processingInstruction(target, StringView.of(data), location, 0);
        } catch (XPathException e) {
            throw new XProcException(e);
        }
    }

The original PI that I'm effectively copying does have a base URI so the SysIdLocation is correct and is being passed to the receiver.

It looks like decompose() in ComplexContentOutputter is copying the PI again and passing a null location, because append in Outputter passes Loc.NONE.

I confess, I don't see where the PI comes into it. It would make more sense if all the elements lost their base URIs.

I'll see if I can extract out a test case.

The bug appears to be something like this: when a tree with no known base URI is copied to a tree builder for which a base URI (system ID) has been set, copying element nodes to the tree causes them to acquire the base URI that was set for the tree builder. However, copying processing instruction nodes causes the corresponding nodes in the result tree to have no base URI; moreover, this change persists until the next node with a known base URI is encountered.

The problem is in TinyBuilder line 534, when handling processing instructions:

        String localLocation = locationId.getSystemId();
        tt.setSystemId(nodeNr, localLocation);

The corresponding code for element nodes is at line 321:

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

where systemId is the systemId property of the TinyBuilder.

I think the code for processing instructions should be the same as for elements.

The fix seems to cause no obvious regression, but I need to create a specific unit test before I'm 100% comfortable with it.

While attempting to create a unit test for this (so far unsuccessfully), I hit an apparently unrelated problem with the TinyBuilder. My test does the following sequence of calls on a TinyBuilder

open()
startDocument()
startElement(TOP)
endElement
documentNode.copy()
startElement(BOTTOM)
endElement
endDocument()
close()

After this sequence of calls, getCurrentRoot() returns the BOTTOM element, not the document node.

What happens here is that the documentNode.copy() operation starts with a startDocument() event, and ends with an endDocument() event. The inner startDocument() event is correctly ignored, but the inner endDocument() is not - it decrements the level number.

After exploring this a little, I concluded that it's not a problem because in practice the events are always sent to the TinyBuilder via a ComplexContentOutputter, which filters out the unwanted startDocument() and endDocument() events.