Project

Profile

Help

Bug #4755

closed

Pattern x/(a|b) is rejected

Added by Michael Kay over 3 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
XSLT Conformance
Sprint/Milestone:
-
Start date:
2020-09-27
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

See bug #4645.

The new tests added for bug #4645 (for Saxon-J) are failing on Saxon-JS: specifically match-266 and match-268.

Actions #1

Updated by Debbie Lockett about 3 years ago

  • Applies to JS Branch 2 added
Actions #2

Updated by Norm Tovey-Walsh about 3 years ago

  • Status changed from New to Resolved

AFAICT, match-265 passes in both NodeJS and the browser.

Actions #3

Updated by Debbie Lockett about 3 years ago

  • Status changed from Resolved to In Progress

Reopening. I believe match-265, match-267, match-269 pass; but match-266 and match-268 still fail.

Actions #4

Updated by Michael Kay about 2 years ago

Looking at match-266, the compiled pattern in the SEF file is clearly incorrect:

{
                           "N": "p.withUpper",
                           "role": "match",
                           "axis": "parent",
                           "sType": "1NE u[NE nQ{}a,NE nQ{}b]",
                           "ns": "= xml=~ fn=~ xsl=~ xs=~ ",
                           "C": [
                              {
                                 "N": "p.venn",
                                 "op": "union",
                                 "C": [
                                    {
                                       "N": "p.nodeTest",
                                       "test": "NE nQ{}a"
                                    },
                                    {
                                       "N": "p.nodeTest",
                                       "test": "NE nQ{}b"
                                    }
                                 ]
                              },
                              {
                                 "N": "p.nodeTest",
                                 "test": "NE nQ{}x"
                              }
                           ]
                        },

because it applies the parent axis (rather than ancestor) to both branches of the union.

SaxonJ handles this (at SlashExpression#843) by rewriting a/(b|c) as (a/b union a/c).

Actions #5

Updated by Michael Kay about 2 years ago

I think the problem is in the Javascript XPath parser code, specifically Step.js line 83, SlashExpression.toPattern(), where the creation of an AncestorQualifiedPattern assumes that the RHS is an AxisExpression. Either there should be special logic here to handle the case where the RHS is a VennExpression, or it should construct a general-purpose GenNodePattern.

Actions #6

Updated by Michael Kay about 2 years ago

I've changed SlashExpression.toPattern() to generate a p.genNode pattern where the RHS is not an AxisExpression. The SEF file looks correct; but the pattern is not matching.

The reason for this is that the x element in the source being matched is parentless. We aren't doing the rewrite of child::x to top::x required by the spec, so the pattern x/a is treated strictly as child::x/child::a; the x doesn't match because it isn't the child of anything.

(Aside: I don't see anything in SaxonJ GeneralNodePattern to handle this either.)

Actions #7

Updated by Michael Kay about 2 years ago

Work on this bug has triggered the construction of a number of new test cases (some of which were also failing in SaxonJ).

At the moment we are failing match-254, match-265, -6, -7, -8, 273, -4, -5, 6,

Actions #8

Updated by Michael Kay about 2 years ago

  • Category set to XSLT Conformance
  • Status changed from In Progress to Resolved
  • Priority changed from Low to Normal
  • Fix Committed on JS Branch 2 added

A number of new pattern tests have been created, and the code has been fixed to make sure they pass.

Actions #9

Updated by Debbie Lockett almost 2 years ago

  • Status changed from Resolved to In Progress

Reopening because I'm seeing differing results for these match tests when running the standard and debug versions of SaxonJS. Indeed the tests pass when running the debug version, but they fail when running the standard version. So this points to a closure compiler renaming issue for the code fix.

Actions #10

Updated by Debbie Lockett almost 2 years ago

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

Found the offending code! In the new copyExpr function in Expr.js, bracket notation must be used to set the "ELAB" property on the new node, to avoid closure compiler renaming.

All match tests now passing with standard version of SaxonJS as well as debug version.

Update committed on main and saxonjs2 branches.

Actions #11

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