Project

Profile

Help

Bug #5036

closed

key() doesn't work on ixsl:page() after ixsl:replace-content?

Added by Martynas Jusevicius over 2 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Normal
Category:
XSLT Conformance
Sprint/Milestone:
-
Start date:
2021-07-06
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

Not sure it's the best subject, but this is what seems to be happening here (see the last 2 templates): https://namedgraph.github.io/saxon-js2-test/client.xsl

Live demo: https://namedgraph.github.io/saxon-js2-test/ Click the [@CLASS KEY] button.

I would expect the lookups to find id('abc', ixsl:page()) (which it does) as well as key('elements-by-class', 'some-class', ixsl:page()) (which it doesn't).

Actions #1

Updated by Martynas Jusevicius over 2 years ago

Using Saxon 2.2.

Actions #2

Updated by Martin Honnen over 2 years ago

I think the problem might simply be that the index for a key declaration is built-once but is not later updated when the document is changed. At https://martin-honnen.github.io/xslt/2021/saxon-js-key-test-ee1.html there is an initial value for the key call but it remains constant while a direct attribute based selection does take the actual state of the tree into account. Solely a guess, however.

Actions #3

Updated by Martin Honnen over 2 years ago

Note that you seem to use Saxon JS 2.1 despite your statement you tested with 2.2. But I don't think that matters.

Actions #4

Updated by Martynas Jusevicius over 2 years ago

You're right, I was still generating SEF with 2.1. Now it should be 2.2 for real.

The key thing is not great then. If ID lookups can work, I would keys expect to work as well. But lets see what Saxonica says :)

As a workaround, I'm trying to call the key on $xhtml to get IDs of those elements and then use id() on ixsl:page().

Actions #5

Updated by Martynas Jusevicius over 2 years ago

The strange thing was, unless I'm mistaken, that if I inlined the some-template code directly into the ixsl:onclick handler, it worked as expected. But I don't want to touch it anymore :)

Actions #6

Updated by Michael Kay over 2 years ago

I do recall a known issue/restriction in this area, but I'm having trouble finding any documentation.

Actions #7

Updated by Martin Honnen over 2 years ago

For accumulators on HTML documents there is a warning: "Accumulator values are attached to DOM nodes in situ. This means a DOM node can't have different values for the same accumulator name in different transformations. This is likely to be a permanent restriction. Advice for users: do not use accumulators on the HTML document, because it is mutable.".

Perhaps the same holds for keys?

Actions #8

Updated by Debbie Lockett over 2 years ago

Martin is right that the key index is only built once, but since the HTML page is mutable this is not necessarily good enough. As Mike and Martin have noted, this restriction is unfortunately not documented. So in the short-term, we need to update the documentation to include a warning about the use of keys for nodes in the HTML page.

In the long term, we should improve the implementation so that such keys are actually usable. The key indexes should be rebuilt if the HTML page has any (relevant) changes.

Actions #9

Updated by Martynas Jusevicius over 2 years ago

Makes sense. MutationObserver could be useful for tracking DOM changes.

Actions #10

Updated by Debbie Lockett over 2 years ago

  • Category set to XSLT Conformance
  • Status changed from New to In Progress
  • Assignee set to Debbie Lockett

Selenium test iss5036 added to our test suite (based on Martin's sample test).

Actions #11

Updated by Debbie Lockett over 2 years ago

Warning added to the documentation, under conformance/xslt30 (which will be updated online with the next maintenance release).

Actions #12

Updated by Martynas Jusevicius about 2 years ago

Any news on this?

How about a new instruction that would rebuild the keys? For example: <ixsl:refresh-key name="lines-by-start"/>

Actions #13

Updated by Martynas Jusevicius about 2 years ago

I had to get rid of key() usages because of this issue and replace them with ixsl:page()//... lookup. Rendering an interactive SVG graph with hundreds of nodes, the performance degradation is clearly noticeable :(

Actions #14

Updated by Michael Kay almost 2 years ago

My inclination is to resolve this by having the key() function do a straight search (without using indexes) when the target document is mutable. The HTML page is rarely going to be so big that it's going to make a big difference.

Actions #15

Updated by Martynas Jusevicius almost 2 years ago

Respectfully, that sounds the worst option of all 3 (the other two being automatic key refresh and explicit refresh instruction). Is it so hard to refresh them keys?

In our case all documents are mutable, because navigation is done by Saxon-JS loading and injecting document fragments into DOM.

As mentioned above, I've replaced key() lookup with // on an SVG document with hundreds of elements, and the slowdown was noticeable.

Actions #16

Updated by Norm Tovey-Walsh almost 2 years ago

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

After some investigation, we concluded that it was easy to force the indexes to be rebuilt after a page change. Starting with the Saxon 2.4 maintenance release, any page change will automatically invalidate the indexes, forcing a rebuild before the next use of the fn:key() function (or other functions that use the indexes).

There’s no performance cost associated with invalidating the indexes. But there is, naturally, a performance cost associated with rebuilding them.

In the (we expect, unlikely) event that you have an application that makes heavy use of indexes, and updates the page, and where the updates are known not to change any of the indexed values, you can disable the behavior.

There is a new API, setConfigurationProperty on the SaxonJS object. If you call:

SaxonJS.setConfigurationProperty('autoResetIndexes', false);

then automatic invalidation of indexes will be disabled. At that point, it becomes the application’s responsibility to reset them. There’s a function on the SaxonJS object to reset the indexes. There’s no ixsl: function in Saxon 2.4, so you have to do it indirectly:

    <xsl:sequence
        select="ixsl:eval('(function(doc) { SaxonJS.resetIndexes(doc) })')
                => ixsl:apply([ixsl:page()])"/>

After the indexes have been reset, any attempt to use an index-backed function will rebuild them.

Actions #17

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