Closed
Bug 96410
Opened 23 years ago
Closed 22 years ago
XPath parser need a context
Categories
(Core :: XSLT, defect, P2)
Core
XSLT
Tracking
()
VERIFIED
FIXED
mozilla1.0
People
(Reporter: sicking, Assigned: axel)
References
Details
(Keywords: arch)
Attachments
(1 file)
6.93 KB,
patch
|
Details | Diff | Splinter Review |
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...
Reporter | ||
Comment 1•23 years ago
|
||
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•23 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
Reporter | ||
Comment 3•23 years ago
|
||
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•23 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•23 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 | ||
Comment 6•23 years ago
|
||
Reporter | ||
Comment 7•23 years ago
|
||
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•23 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
Comment 9•23 years ago
|
||
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•23 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?
Reporter | ||
Comment 11•23 years ago
|
||
<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
Comment 12•23 years ago
|
||
FunctionCall* getExtensionFunction(txAtom* prefix, PRInt32 nsID); Why the prefix and the nsID. Won't nsID be enough?
Comment 13•23 years ago
|
||
and don't you need the function name? ;)
Reporter | ||
Comment 14•23 years ago
|
||
sorry, s/prefix/name/
Assignee | ||
Comment 15•23 years ago
|
||
pushing out. discussion goes by email at the moment.
Keywords: arch
Target Milestone: mozilla0.9.6 → mozilla0.9.8
Assignee | ||
Comment 16•22 years ago
|
||
these bugs should go in with bug 113611
Comment 17•22 years ago
|
||
So can we close this now?
Assignee | ||
Comment 18•22 years ago
|
||
fixed with checkin of bug 113611.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 19•22 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.
Description
•