From Bugzilla Helper: BuildID: 2002032404 With an XSLT fragment like this: <xsl:for-each select="ele"> <xsl:sort select="@attr"/> <xsl:value-of select="position()"/> </xsl:for-each> The position() function should return values in order 1, 2, 3 and so on, no matter what the sort does. Currently, it returns values with no apparent pattern. Reproducible: Always Steps to Reproduce: 1. Put the attached position.xml and position.xsl in the same directory 2. Open position.xml Actual Results: It shows 1 0 Expected Results: Correct is 1 2 Explanation: XPath 1.0 para 4.1 says: |The position function returns a number equal to the context position |from the expression evaluation context. XSLT 1.0 para 4 says: [when evaluating XPath expressions] |the context position comes from the position of the current node in |the current node list; the first position is 1 XSLT 1.0 para 10 says: [Inside <xsl:sort>] |the current node list list (sic) consists of the complete list of |nodes being processed in sorted order This worked correctly with Mozilla 0.9.8. It was wrong in Mozilla 0.9.9. I've reproduced it on both GNU/Linux and Windows. The build-id above is actually from the Debian mozilla-snapshot package. With more complicated testcases, the position() values are irregular, but frequently zero.

uhoh, this is a regression, doing NodeSet::indexOf using NodeSet::findPosition won't work for non-documentorder-sorted nodesets! This specific testcase will probably be fixed by Pikes branch, but i'm not sure if there could be any other cases where we'll fail anyway

for the record, it's not the for-each that is giving us trouble, it's the sorting.

two-fold problem: - indexOf() is broken for our NodeSet impl if content isn't sorted in document order. - position() returns wrong number for xsl:sort. The first one is an architecture bug for our nodesets, the second is more or less a problem of the *right* context. Fix for second is part of bug 113611. (tested on standalone) (adding deps, adjusting topic)

for now i think we should do this. It might regress some uses of attribute-nodes, but the current brokenness is IMHO far worse

taking since I caused the regression and i have the patch

Comment on attachment 81780 [details] [diff] [review] temporary workaround Fix the comment ("attribute-nodes arn't broken"). r=peterv

Comment on attachment 81780 [details] [diff] [review] temporary workaround checked in on trunk

Peter, do you think this would be serious enough for Netscape 7.0 RTM? What does/could the patch break?

No point in keeping this open since it won't make the branch.

Huh? Mozilla 1.0.1 is long ways out, so this certainly has a chance to get in. Make your case why we need this and we'll consider.

/me realizes there is a life after 1.0 :-)

Comment on attachment 81780 [details] [diff] [review] temporary workaround Please land this on the 1.0.1 branch. Once there, remove the "mozilla1.0.1+" keyword, and add the "fixed1.0.1" (This approval is based on the comment in sicking's email: "The patch backs out part of the patch from bug 85893 which caused a pretty serious regression in the XPath engine.")

checked in to branch

we didn't verify for a long time. I really checked, so VERIFIED.