Closed Bug 96410 Opened 23 years ago Closed 22 years ago

XPath parser need a context

Categories

(Core :: XSLT, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.0

People

(Reporter: sicking, Assigned: axel)

References

Details

(Keywords: arch)

Attachments

(1 file)

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: 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...
"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...
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.
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
Attached patch first phase of context (deleted) — Splinter Review
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"/>
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.
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? ;)
pushing out. discussion goes by email at the moment.
Keywords: arch
Target Milestone: mozilla0.9.6 → mozilla0.9.8
No longer blocks: 110266
Depends on: 113611
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?
fixed with checkin of bug 113611.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
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.

Attachment

General

Creator:
Created:
Updated:
Size: