XPath parser need a context

VERIFIED FIXED in mozilla1.0

Status

()

Core
XSLT
P2
normal
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: sicking, Assigned: Axel Hecht)

Tracking

({arch})

Trunk
mozilla1.0
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

To get rid of the "resolve namespaces/baseURI at runtime rather then parsetime" 
problem we need to have some sort of parse-context during parsing that can be 
used to resolve prefix->namespace and get current base uri to resolve relative 
urls against. The same context could also be used to report parseerrors (which 
we currently don't do).

The interface must also be possible to use for standalone XPath

I propose something like this:

class txXPathParseContext {
public:
    PRInt32 resolveNamespacePrefix(txAtom* prefix);
    String  getBaseURI();

    FunctionCall* getExtensionFunction(const String& name, PRInt32 nsID);

    void    recieveError(String msg);
};

or some such...
Depends on: 95779
(Assignee)

Updated

17 years ago
Depends on: 76070
hmm.. or make getExtensionFunction use txAtom instead of String perhaps... 
Should speed up but somewhat complicate implementors, but it's probobly the way 
to go anyway.

Hmm.. come to think of it, we should proboly have some "ExpandedName" class 
that consists of a namespace-id, a local-name-atom (and perhaps an optional 
prefix-atom). There are actually a lot of places where we just do 
stringcomparisons where we actually should compare expanded names, variables 
for example.

Not really required for this bug, but something to thing about...
(Assignee)

Comment 2

17 years ago
"Parse" is in discussion.

I propose three steps:

Get a ContextState into the public ExprParser methods. (simple, done in my tree)
'atomize' the ContextState/ProcessorState interfaces (bug 76070)
split ProcessorState into a XPath only state, and a XSLT state inheriting
from that.

Each should land independant.

Axel
Isn't ContextState the "XPath only state"?

Also, who should implement these functions. IMHO |getExtensionFunction| belong 
in XSLTProcessor, and the rest in ProcessorState. However many of the functions 
returned from |getExtensionFunction| need a handle to the ProcessorState or 
some stuff in it, so it seems a bit silly to forward calls from ProcessorState 
to XSLTProcessor and then use a bunch of functions to dig back into 
ProcessorState...
(Assignee)

Comment 4

17 years ago
I was trying to say, split the *implementation* into a XPath and a XSLT part.

As to resolveFunctionCall, <fantasy> if we could register additional extension
functions, those would be part of the ProcessorState </fantasy>, so there
is a reason for having resolveFunctionCall in ProcessorState.
Even if the reason is more like a bunch of flowers changing their colours as 
you speak.
(Assignee)

Comment 5

16 years ago
I say phase one of this is blocking bug 102293. I can't see the rest of the
deps in today's state of mind.

I will attach a patch which gives the expr parser a mContext, so we can start
doing good things on parsings. Like prefix resolving and error reporting.

Axel
Assignee: kvisco → axel
Blocks: 102293
No longer depends on: 76070, 95779
OS: Windows 2000 → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla0.9.6
(Assignee)

Comment 6

16 years ago
Created attachment 53121 [details] [diff] [review]
first phase of context
we can't do any namespace resolution at parsetime until bug 95779 is checked 
in. As long as we cache expressions on string there is the risk that an 
expression is parsed in a context with one set of namespace mappings and used 
in a context with another set of namespace mappings. Consider these two 
elements in a stylesheet (or in separate stylesheets but imported by the same 
stylesheet)

 <xsl:apply-templates select="ns:elem" xmlns:ns="some-namespace"/>

 <xsl:apply-templates select="ns:elem" xmlns:ns="some-other-namespace"/>
(Assignee)

Comment 8

16 years ago
I'd happily break in that respect. We should move forward on those fronts that
are rather easily to move. I'm afraid we gonna run into circular deps soon.
Let's just make a cut here.
This is an edge case, and we know that we're gonna fix it. So I'm all for
landing this now, and to get the love to bug 102293.

Axel
Two quick comments: a) I need to be able to pass in a NSResolver for DOM Level 3
XPath to be used when parsing (see
http://www.w3.org/TR/2001/WD-DOM-Level-3-XPath-20010830/xpath.html#XPathNSResolver)
and b) Extension functions are coming, i have a prototype almost finished.
(Assignee)

Comment 10

16 years ago
I think the parsing context should be ErrorObserver and
virtual FunctionCall* resolveFunctionCall(const String& name) = 0;
virtual void getNameSpaceURIFromPrefix(const String& prefix, String&
nameSpaceURI) = 0;

This is currently most closely ContextState, but I might get talked into 
stripping that down.

How about killing NamespaceResolver down to getNameSpaceURIFromPrefix?
I don't see that any of 
getResultNameSpaceURI or
getNameSpaceURI
is called anyway. Peterv, does that stay that way?
<Sidetrack>
To fix bug 96802 the parsecontext need a function like

NamespaceResolver* getNamespaceResolver();

This namespaceresolver has to work even after parsing, so it can't be the 
ProcessorState since the ProcessorState will change it's context-node. Strictly 
speaking we don't need it since only xslt-functions need to do qname-lookups, 
but it'd be nice to have a more generic solution.

However I think we should maybe wait with adding this function until the fix 
for bug 96802.
</Sidetrack>

I think we should have separate interfaces for parsing and evaluation, 
something like

class txXPathParseContext {
public:
    PRInt32       resolveNamespacePrefix(txAtom* prefix);
    String        getBaseURI();
    NamespaceResolver* getNamespaceResolver();
    FunctionCall* getExtensionFunction(txAtom* prefix, PRInt32 nsID);
    void          recieveError(String msg, ErrorLevel level);
};

class txXPathEvalContext {
public:
    ExprResult* getVariable(String& name);
    NodeSet*    getCurrentNodeSet();
    void        setCurrentNodeSet(NodeSet*);
    MBool       isStripSpaceAllowed(Node* node);
    void        recieveError(String msg, ErrorLevel level);
};

However we will probobly have to morph the current eval-context into the 
desired one so that we can do this in stages.

Peter: You should be able to do both DOM3-namespaceresolving and extension 
functions with the above interfaces
 FunctionCall* getExtensionFunction(txAtom* prefix, PRInt32 nsID);

Why the prefix and the nsID. Won't nsID be enough?
and don't you need the function name? ;)
sorry, s/prefix/name/
(Assignee)

Comment 15

16 years ago
pushing out. discussion goes by email at the moment.
Keywords: arch
Target Milestone: mozilla0.9.6 → mozilla0.9.8
Blocks: 110266
(Assignee)

Updated

16 years ago
No longer blocks: 110266
Blocks: 116534
No longer blocks: 116534
Blocks: 116534
(Assignee)

Updated

16 years ago
Depends on: 113611
(Assignee)

Comment 16

16 years ago
these bugs should go in with bug 113611
Status: NEW → ASSIGNED
Keywords: mozilla1.0
Target Milestone: mozilla0.9.8 → mozilla1.0
So can we close this now?
(Assignee)

Comment 18

16 years ago
fixed with checkin of bug 113611.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 19

16 years ago
we didn't verify for a long time.
I really checked, so VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.