Closed Bug 405069 Opened 15 years ago Closed 15 years ago

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

Categories

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

x86
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.9beta2

People

(Reporter: WeirdAl, Assigned: WeirdAl)

References

Details

(Keywords: assertion, regression, testcase)

Attachments

(2 files)

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.
I should mention this was on a completely fresh Firefox 3.0b1 source checkout, no changes whatsoever.  Fedora 8 Linux.
Attached file 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)]
also seen on trunk, and on Windows
OS: Linux → All
Version: unspecified → Trunk
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.
Assignee: jonas → peterv
Blocks: 402442
Blocks: 402422
No longer blocks: 402442
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
Flags: blocking1.9-
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.
(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.
Look at the documentation for the IsNodeOfType function
Attached patch patchSplinter Review
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.
Attachment #290650 - Flags: superreview?(jonas)
Attachment #290650 - Flags: review?(jonas)
No xpcshell test that fails when the relevant assertion is hit in an unpatched build?
Flags: in-testsuite?
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.
Attachment #290650 - Flags: superreview?(jonas)
Attachment #290650 - Flags: superreview+
Attachment #290650 - Flags: review?(jonas)
Attachment #290650 - Flags: review+
Attachment #290650 - Flags: approval1.9?
Attachment #290650 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Assignee: peterv → ajvincent
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
Status: NEW → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M10
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
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.