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)
(deleted),
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•23 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
•