405069 - Running xpathgen tests: ###!!! ASSERTION: called nsGenericElement::TextLength: 'Not Reached', file /home/ajvincent/beta/mozilla/content/base/src/nsGenericElement.cpp, line 3971

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Alex Vincent [:WeirdAl]]

Steps to reproduce:
(1) ac_add_options --enable-extensions=default,xpath-generator
(2) Build Firefox.
(3) make -C extensions/xpath-generator check

No stack trace available yet (gdb couldn't generate a useful one).

This definitely wasn't happening when I first checked in XPath Generator code.

Tentatively assigning to sicking (cvs blame says the assertion was added by him) until I can debug this.

Comment 1

[Alex Vincent [:WeirdAl]]

I should mention this was on a completely fresh Firefox 3.0b1 source checkout, no changes whatsoever.  Fedora 8 Linux.

Comment 2

[Alex Vincent [:WeirdAl]]

Attachment: stack trace

So the node at the top of the stack is really a document fragment, and not an element.  It's coming from an XML element, which I can say with confidence is the first child of the document fragment.

1 runSingleTest(i = 50, j = 51, searchFlags = 0) ["../../../_tests/xpcshell-simple/test_xpathgen//unit/test_bug319768.js":187]
    targetNode = [xpconnect wrapped (nsISupports, nsIDOMDocumentFragment, nsIDOMNode, nsIDOM3Node) @ 0x17a6878 (native @ 0x17a67b0)]
    contextNode = [xpconnect wrapped (nsISupports, nsIDOMElement, nsIDOMNode) @ 0x17a6d28 (native @ 0x17a6cd8)]
    nodesList = [... a very long array ...]
    path = ".."
    status = "success"
    failureReport = null
    err = null
    positionNode = [xpconnect wrapped (nsISupports, nsIDOMDocumentFragment, nsIDOMNode, nsIDOM3Node) @ 0x17a6878 (native @ 0x17a67b0)]
    compareNode = [xpconnect wrapped (nsISupports, nsIDOMElement, nsIDOMNode) @ 0x17a6d28 (native @ 0x17a6cd8)]
    UNORDERED_NODE = 8
    expr = [xpconnect wrapped nsIDOMXPathExpression @ 0x1802380 (native @ 0x1802288)]
    xpathResult = undefined
    checkNode = undefined
    this = [object BackstagePass @ 0xde2328 (native @ 0xdcafd4)]

Comment 3

[Alex Vincent [:WeirdAl]]

also seen on trunk, and on Windows

Comment 4

[Alex Vincent [:WeirdAl]]

peterv, jst:  According to the stack for this bug, nsXPathResult::SetExprResult calls mNumberResult = mResult->numberValue() - which it didn't do before bug 402422.  I believe this is the cause of the assertion firing now.

That said, the test is essentially checking to see if we can step from a XML element inside a document fragment to that element.  XPath (the specification) doesn't explicitly cover the case of DocumentFragments as targets (see bug 402129 for the context node case, where they are explicitly disallowed), at least upon first glance.

So there may be two bugs.  (1) That the assertion triggers, and (2) allowing a XPath to evaluate to a document fragment.

Reassigning to peterv for further work.  I'm not sure if this should be wanted-1.9, but I don't feel I can justify it... I hit this on a very edge-case scenario, in a test for a NPODB extension.

Comment 5

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

We need to add a check for aNode.mNode->IsNodeOfType(nsINode::eDOCUMENT_FRAGMENT) to

http://lxr.mozilla.org/mozilla/source/content/xslt/src/xpath/txMozillaXPathTreeWalker.cpp#580

Patches accepted, but this isn't blocking 1.9

Comment 6

[Alex Vincent [:WeirdAl]]

Hm, I guess it's not quite that easy.  When I changed the line to say:

        aNode.mNode->IsNodeOfType(nsINode::eELEMENT |
                                  nsINode::eDOCUMENT_FRAGMENT)) {

I still failed the assertion.

Comment 7

[Peter Van der Beken [:peterv]]

(In reply to comment #6)
>         aNode.mNode->IsNodeOfType(nsINode::eELEMENT |
>                                   nsINode::eDOCUMENT_FRAGMENT)) {

Read comment 5 again, it's not what Jonas told you to do. A node can't be both an element and a document fragment at the same time.

Comment 8

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

Look at the documentation for the IsNodeOfType function

Comment 9

[Alex Vincent [:WeirdAl]]

Attachment: patch

Reading the docs behind IsNodeOfType is what got me confused in the first place... this was what I thought I should do until I read them.  :)

This patch works, by the way.

Comment 10

[Jeff Walden [:Waldo]]

No xpcshell test that fails when the relevant assertion is hit in an unpatched build?

Comment 11

[Alex Vincent [:WeirdAl]]

jwalden: see comment 0 - I put steps to reproduce there which don't need patching... just a slightly different MOZCONFIG.

That said, it should be pretty easy to write a test for this, if you want to.  I'm not personally convinced it's worth it; this is an edge case that I hit.

I'd do an unofficial review on any such test someone else wrote, though - particularly if it tested far more than just the assertion.  I don't know what we have in the way of automated XPath tests.

Comment 12

[Reed Loden [:reed]]

Checking in content/xslt/src/xpath/txMozillaXPathTreeWalker.cpp;
/cvsroot/mozilla/content/xslt/src/xpath/txMozillaXPathTreeWalker.cpp,v  <--  txMozillaXPathTreeWalker.cpp
new revision: 1.29; previous revision: 1.28
done

Comment 13

[Reed Loden [:reed]]

Relanded.

Checking in content/xslt/src/xpath/txMozillaXPathTreeWalker.cpp;
/cvsroot/mozilla/content/xslt/src/xpath/txMozillaXPathTreeWalker.cpp,v  <--  txMozillaXPathTreeWalker.cpp
new revision: 1.31; previous revision: 1.30
done