72810 - count() function is unable to handle certain path expressions

Status: VERIFIED FIXED

(Core :: XSLT, defect, P2)

Description

[Walter Schmid]

See attached XML/XSL files.

Comment 1

[Walter Schmid]

Attachment: Simple XML input

Comment 2

[Walter Schmid]

Attachment: XSL input file

Comment 3

[Walter Schmid]

Attachment: Expected output

Comment 4

[Axel Hecht]

This works fine up to the point where we try to evaluate the argument to the 
count() function.
The reason for this problem seems to be the context node, which is the
xsl:variable node. Heck, that's part of the stylesheet, that can't be right, hu?

But what is the right context node? This is called from processTopLevel, and I
poked the spec without any luck about what the context node should be, as we
don't instantiate any template yet. Should we take the source doc in this case?

stack trace: (for a slightly modified version, using a select attribute)

#0  NodeSetFunctionCall::evaluate (this=0xb1e00, context=0xb4fa0, 
    cs=0xffbef3c0)
    at
/home/ah/source/XSLT/thirdtree/mozilla/extensions/transformiix/source/xpath/NodeSetFunctionCall.cpp:95
#1  0x87598 in XSLTProcessor::processVariable (this=0xffbef8b0, node=0xb4fa0, 
    xslVariable=0xb4fa0, ps=0xffbef3c0)
    at
/home/ah/source/XSLT/thirdtree/mozilla/extensions/transformiix/source/xslt/XSLTProcessor.cpp:1603
#2  0x8268c in XSLTProcessor::processTopLevel (this=0xffbef8b0, 
    stylesheet=0xb1da0, ps=0xffbef3c0)
    at
/home/ah/source/XSLT/thirdtree/mozilla/extensions/transformiix/source/xslt/XSLTProcessor.cpp:493
#3  0x81dc4 in XSLTProcessor::processTopLevel (this=0xffbef8b0, 
    xslDocument=0xb14f8, ps=0xffbef3c0)
    at
/home/ah/source/XSLT/thirdtree/mozilla/extensions/transformiix/source/xslt/XSLTProcessor.cpp:379
#4  0x82c94 in XSLTProcessor::process (this=0xffbef8b0, xmlDocument=@0xaf458, 
    xslDocument=@0xb14f8, out=@0xab8c0)
    at
/home/ah/source/XSLT/thirdtree/mozilla/extensions/transformiix/source/xslt/XSLTProcessor.cpp:595
#5  0x8329c in XSLTProcessor::process (this=0xffbef8b0, xmlInput=@0xae938, 
    xmlFilename=@0xae318, xslInput=@0xffbef780, xslFilename=@0xae330, 
    out=@0xab8c0)
    at
/home/ah/source/XSLT/thirdtree/mozilla/extensions/transformiix/source/xslt/XSLTProcessor.cpp:680
#6  0x8c58c in main (argc=5, argv=0xffbef98c)
    at
/home/ah/source/XSLT/thirdtree/mozilla/extensions/transformiix/source/main/transformiix.cpp:135

Comment 5

[Axel Hecht]

doing milestone, and partially keywords that didn't survive midair. My 
stacktrace wins ;-)

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This should be fixed with the new parser

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

ignore me :(

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The context node and context node list should be the rootnode of the source 
document per http://www.w3.org/TR/xslt#top-level-variables

Comment 9

[Keith Visco]

Yep XSLTProcessor::processTopLevel is definately wrong, the source document node 
should be passed in as a parameter:

something like:

processTopLevel(Document* source, Document* stylesheet, ProcessorState* ps);

-and-

processTopLevel(Document* source, Element* stylesheet, ProcessorState* ps);

then lines 423 and 503 both need to be changed to:

ExprResult* exprResult = processVariable(source, node, ps);

and line 565 should be changed to:

processTopLevel(&xmlDocument, &xslDocument, &ps);

That should fix it.

--Keith

Comment 10

[Asa Dotzler [:asa]]

Are you still shooting for 0.9 on this? If so please email [e-mail]
with a status on you progress. If not please retarget against a later Milestone.
Thanks.

Comment 11

[Axel Hecht]

retargeting.

Axel

Comment 12

[chris hofmann]

is this on track for landing in the next week or so?
if not lets unset the target milestone until we get a good plan in place.

Comment 13

[Peter Van der Beken [:peterv]]

I'll try out the suggestion and make a patch.

Comment 14

[Peter Van der Beken [:peterv]]

Attachment: Fix

Comment 15

[Peter Van der Beken [:peterv]]

I just copied what Keith wrote (with some clean-up). Seems to work :).

Comment 16

[Peter Van der Beken [:peterv]]

Attachment: Same fix but pass element to ProcessTopLevel

Comment 17

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

We need both processTopLevel functions, the one that takes an element is needed 
for embedded stylesheets (spec section 2.7) and the one that takes a document 
is a great place to support LRE stylesheets (spec section 2.3)

Also would you mind setting the context nodeset and xslt-current node so that 
we get the following variables right?
<xsl:variable select="position()"/>
<xsl:variable select="last()"/>
<xsl:variable select="current()"/>
The context nodeset should be a nodeset containing only the root-node of the 
source document

Comment 18

[Axel Hecht]

/me tries to scare peterv.
Could a embedded stylesheet be a 2.3 LRE? It doesn't say so in 2.7, but ???

Axel

Comment 19

[Peter Van der Beken [:peterv]]

Attachment: Third try - I ignore section 2 BTW ;)

Comment 20

[Axel Hecht]

r=me, once you add NS_ASSERTION for aStylesheet, too.
The documentElement stuff can be done either here, or in 85408, whichever

Axel

Comment 21

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

ugh, I hate to do this, but I still have a complaint :(

Why do the push/pop for every variable and not just once around the initial 
call to processTopLevel. That way we would also get the right current nodelist 
for the initial templatematching.


Also, it's the sourcedocs root-node that should be the current node and context 
nodelist, not it's documentelement. 

Comment 22

[Peter Van der Beken [:peterv]]

Attachment: Fourth try

Comment 23

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

/me slaps peterv

what part of "root-node" don't you understand ;)
instead of
+    sourceRoot = sourceDocument->getFirstChild();
+    NS_ASSERTION(sourceRoot, "source document has no root node");
+    if (sourceRoot) {
+        nodeSet.add(sourceRoot);
+        ps->pushCurrentNode(sourceRoot);
+    }

just do

+    nodeSet.add(sourceDocument);
+    ps->pushCurrentNode(sourceDocument);

:)

other then that, r=sicking

Comment 24

[Peter Van der Beken [:peterv]]

:). Right, right. I think I'll go have some sleep now.

Comment 25

[Peter Van der Beken [:peterv]]

Attachment: Final patch

Comment 26

[Johnny Stenback (:jst)]

sr=jst

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

a=roc on behalf of drivers

Comment 28

[Peter Van der Beken [:peterv]]

Checked in.

Comment 29

[Axel Hecht]

bitching buttons, verfication spam