[BUG] Nodeset extension or SimpleNodeLocator.stepPattern

[Gary L Peskin <ga...@firstech.com>]

Developers, _please_ read this.

With help from Greg Keraunen, I have surfaced a small problem in the
nodeset extension. The nodeset extension works by creating a special
DocumentFragment.  This special DocumentFragment reports its type as a
Document so that it can participate in XPath expressions and patterns
just like any other node.

However, org.apache.xalan.xpath.SimpleNodeLocator.stepPattern actually
tries to cast the thing to a Document and the cast fails with a
ClassCastException because the special DocumentFragment doesn't
implement Document.  To implement Document requires adding an additional
17 dummy methods.

As an alternative, changing the following code in stepPattern seems to
alleviate the problem.  The original code is:

case XPath.FROM_ROOT:
  {
    argLen = xpath.getArgLengthOfStep(opPos);
    opPos = xpath.getFirstChildPosOfStep(opPos);
    Document docContext = (Node.DOCUMENT_NODE == context.getNodeType()) 
                          ? (Document)context :
context.getOwnerDocument();
    score = (docContext.equals( context )) ? XPath.MATCH_SCORE_OTHER :
XPath.MATCH_SCORE_NONE;
    if(score == XPath.MATCH_SCORE_OTHER)
    {
      context = docContext;
    }
  }
  break;

To me, this code is confusing anyway due to the two ternary operator
statements in a row that basically test for the same condition followed
by an if statement that does the same thing.  In addition, the "context
= docContext" just assigns an object back to itself for no apparent
reason (I think).  I propose the following change:

case XPath.FROM_ROOT:
  {
    argLen = xpath.getArgLengthOfStep(opPos);
    opPos = xpath.getFirstChildPosOfStep(opPos);
    if (Node.DOCUMENT_NODE == context.getNodeType())
    {
      score = XPath.MATCH_SCORE_OTHER;
    }
    else
    {
      score = XPath.MATCH_SCORE_NONE;
    }
  }
  break;

Or the more concise alternative:

case XPath.FROM_ROOT:
  {
    argLen = xpath.getArgLengthOfStep(opPos);
    opPos = xpath.getFirstChildPosOfStep(opPos);
    score = (Node.DOCUMENT_NODE == context.getNodeType()) ?
               XPath.MATCH_SCORE_OTHER : XPath.MATCH_SCORE_NONE;
  }
  break;


This avoids the need for the cast, results in (I think) cleaner code,
and avoids the need to create the docContext intermediate value.  Also,
nodeset will now work as is without changes.

If this is okay with everyone, I'll go ahead and prepare the version 1
and version 2 diffs using the last bit of code unless there is an
objection to that.

Thanks,
Gary