Closed
Bug 73936
Opened 24 years ago
Closed 23 years ago
xsl:include/xsl:import/document() doesn't work
Categories
(Core :: XSLT, defect, P1)
Core
XSLT
Tracking
()
VERIFIED
FIXED
mozilla0.9.4
People
(Reporter: markushuebner, Assigned: peterv)
References
()
Details
(Whiteboard: ns6.1)
Attachments
(6 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
no display of content elements - works fine in MSIE :-/
Reporter | ||
Updated•24 years ago
|
Severity: normal → blocker
Updated•24 years ago
|
Severity: blocker → normal
OS: Windows 2000 → All
Hardware: PC → All
Comment 2•24 years ago
|
||
The resulting doc contains CDATA in the <head>, which is not allowed.
So marking this invalid.
Markus, we don't do IE-hacky serialization-parsing stuff. Your output has to be
the TREE you expect, not the serialized source.
Axel
Status: UNCONFIRMED → RESOLVED
Closed: 24 years ago
Resolution: --- → INVALID
Reporter | ||
Comment 3•24 years ago
|
||
Removed the CDATA in the head (which hasn't been a problem at all until now -
how should the JavaScript in the HEAD be placed then?!) and the bug still
remains.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Comment 4•24 years ago
|
||
WORKSFORME
Platform: PC
OS: Windows 98
Mozilla build: 2001040404
Download the latest nightly and see if that fixes the problem.
Comment 5•24 years ago
|
||
'k, now we're getting closer.
Markus, why does that testcase still have another external stylesheet?
I don't think for a reason, 'cause the standalone processor ate that fine.
Confirming, shouldn't too bad to track down. Adding a few CCs and helpwanted.
Axel
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: helpwanted
Summary: No display of content → no content generated for CDATASection
Reporter | ||
Comment 6•24 years ago
|
||
so the external stylesheet isn't resolved correctly ...
Severity: normal → major
Reporter | ||
Updated•24 years ago
|
Summary: no content generated for CDATASection → xsl:include doesn't work
AFAICT there is a regression in the SyncLoader. It seems it is unable to load
any documents. This is also the cause of all redness in buster even with
Heikkis fix for bug 45377
Assigning to peterv
Assignee: kvisco → peterv
Keywords: regression
This bug is now our official Syncloader-dosn't-work bug
Priority: -- → P2
Target Milestone: --- → mozilla0.9.1
peterv thinks this is related to bug 71895
Assignee | ||
Updated•24 years ago
|
Target Milestone: mozilla0.9.1 → mozilla1.1
Reporter | ||
Comment 10•24 years ago
|
||
This bug has a major impact on the functionality of XSLT since includes are
commonly used among web developers. I think this one should have a high
priority and should be fixed as soon as possible - target 0.9.1
Assignee | ||
Comment 11•23 years ago
|
||
changing summary
Summary: xsl:include doesn't work → xsl:include/xsl:import/document() doesn't work
Assignee | ||
Comment 12•23 years ago
|
||
*** Bug 81124 has been marked as a duplicate of this bug. ***
Comment 13•23 years ago
|
||
target milestone 1.1 for this regression bug? come on, this is much more
important, i think xslt is much less fun or usefull in a browser without any way
to include other documents.
sorry for the spam, can't really help you fixing this, but i feel this is an
important bug to fix rather soon.
Assignee | ||
Comment 14•23 years ago
|
||
I know this bug is important. It's also hard to fix. The milestone represents my
current estimate of when I'll have a solution. If I can fix it earlier, I will.
I suggest you look into server-side includes as a workaround for now.
Assignee | ||
Comment 15•23 years ago
|
||
Pulling this back. I think I have a fix, but I'll need someone with experience
with Necko and threads to look at it.
Status: NEW → ASSIGNED
Priority: P2 → P1
Target Milestone: mozilla1.1 → mozilla0.9.2
Assignee | ||
Comment 16•23 years ago
|
||
Comment 17•23 years ago
|
||
This patch should include the renaming of nsLoadListenerProxy to
txLoadListenerProxy, IMHO.
Axel
Comment 18•23 years ago
|
||
adding gagan on CC for some netwerk help,
gagan, is this the right (or even a good) way to load synchronously?
Anyone else we should ask?
Axel
nsBranch ok, limited to XSLT only.
Assignee | ||
Comment 20•23 years ago
|
||
Assignee | ||
Comment 21•23 years ago
|
||
Ok, looking to get this one reviewed and super-reviewed. It fixes document
loading and also adds a security check to document loading from XSLT stylesheets.
Keywords: helpwanted → review
Whiteboard: Need r=, sr=
Comment 22•23 years ago
|
||
Why leave the #if 0 blocks in nsSyncLoader::~nsSyncLoader() and
nsSyncLoader::LoadDocument()?
The event queue/modality changes look really really frightening, does that work
in non-mozilla apps that embed gecko? Make sure that works in mfcembed and
gtkembed before checking in.
Other than that, sr=jst
The changes to DocumentFunctionCall is not right.
First of all you it's not really correct to use the |currentAction->node| as
|aXSLTElement| when creating the DocumentFunctionCall object. The reason for
this is that we hash our expressions keyed on string, so if the same expression
appears at another place in the xslt-document the same objecttree and thus the
same (=wrong) aXSLTElement is used. OTOH the current way to just use
|xslDocument| is also wrong so... The RightThing to do is to add a
ContextState::getBaseUri function that returns currentAction->node->getBaseURI,
since this would be evaluated at evaluation-time (when currentAction is always
up-to-date) rather then at parse-time. And simply drop the
aXSLTElement/xslDocument argument to DocumentFunctionCall. Sorry for not doing
this when I did my original cleanup of this code, but I didn't know enough
about transformiix at the time...
Second, the |if (uri.isEqual("")) {xmlDoc = mXSLTElement->getOwnerDocument();}|
is wrong. The base uri dosn't neccesarily point to the current document, I
would expect a the following two expressions to return the same thing:
"document('foo.xml')"
"document('', 'foo.xml')"
IMHO add logic for uri/baseuri being "" in URIUtils::resolveHref
oh, and would you mind fixing the of-by-one error in sortByDocumentOrder if
you're fixing up that line anyway
Just to clarify myself: I'm ok with using currentAction->node, but leave the
|XXX TODO| comment. However I'd prefer the RightThing by adding a
ContextState::getBaseURI() method.
oh, isn't there a risk that currentAction is NULL if document() appears in a
top-level variable?
Comment 26•23 years ago
|
||
I took over to include the comments, as peterv is on vacation.
What I did:
I made "" resolve OK for href and baseURI in URIUtils.cpp. This is what RFC 1808
wants, there might be a bug in the mozilla code that we called.
I paired the call to AddEventListenerByIID. If you addref, release ;-).
We shouldn't leak the txLoadListener.
I moved the order of the code a bit, to get along with the bunch of early
returns.
Added a null check for mXMLContext and currentAction.
Removed the special casing of "", this is handled in URIUtils now.
Added a comment to the #if 0 code.
Added a debug only fprintf(stderr,...) for document loading. Need more work in
a separate bug to get this to xpcom logging.
Yes, the hashing is broken, but the right way to fix this is parsing XPath with
context, we need to fix that in a different bug.
Tested on gtkEmbed, nisheeth volunteered to be win buddy for mfcEmbed
Patch coming up
Axel
Comment 27•23 years ago
|
||
My ISP has me cut off currently, so I can't test the patch. However after
looking at it I say:
r=sicking on the non-syncloader stuff
(I don't grok the syncloader)
Comment 29•23 years ago
|
||
Don't you need changes to nsXULDocument to prevent it from having pure virtual
methods?
Assignee | ||
Comment 30•23 years ago
|
||
Good catch David. Actually, I forgot to remove the SetDocumentURL method from
the nsIXULDocument interface (which inherits from nsIDocument). New patch coming
up with some other fixes. Looking for r and sr again.
Assignee | ||
Comment 31•23 years ago
|
||
Assignee | ||
Comment 32•23 years ago
|
||
And i tested it with MFCEmbed, WinEmbed and PPEmbed.
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Assignee | ||
Comment 33•23 years ago
|
||
Comment 34•23 years ago
|
||
remove the
+ // See if we're trying to load the stylesheet
+ xmlDoc = xsltElement->getOwnerDocument();
+ xmlDoc->getBaseURI();
hunk from DocumentFunctionCall::retrieveDocument, please.
Other than that, r=me.
The code is way better than it was before, we have found a great deal of bugs
while Peter went thru a great deal of pain.
Axel
Whiteboard: Need r=, sr= → r=axel@pike.org, Need sr=
really sorry but I've got additional pain:
remove the
+ Node* xsltElement;
+
+ xsltElement = mProcessorState->peekAction();
+ if (!xsltElement) {
+ // no xslt element
+ return;
+ }
hunk too (same place as Pikes hunk)
why have the mMainStylesheetURL, why not call xslDocument->getBaseURI() each
time?
Also, if you feel like it, the mMainStylesheetURL thingy is needed
for "mMainSourceDocURL" aswell...
Other then that, r=me on except on syncloader which I still don't grok
Assignee | ||
Comment 36•23 years ago
|
||
Comment 37•23 years ago
|
||
Peter sayz:
hunk is needed
xmlDoc = xmlParser.getDocumentFromURI(absUrl, "",
xsltElement->getOwnerDocument(), errMsg);
member for source doc mMainSourceURL added
Jonas sayz:
go!
Axel
Whiteboard: r=axel@pike.org, Need sr= → r=axel@pike.org,sicking@bigbucks.com, Need sr=
Comment 38•23 years ago
|
||
sr=jst
Assignee | ||
Comment 39•23 years ago
|
||
Aaaaaaaaah.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 40•23 years ago
|
||
Unfortunately the testcase at
http://www.world-direct.com/gia/test/mozillatest.asp
still doesn't display correctly.
Reporter | ||
Comment 41•23 years ago
|
||
tried again with a later build - 2001080303 and everything is working fine
now :)
Comment 42•23 years ago
|
||
verified, per comments of reporter and myself
Status: RESOLVED → VERIFIED
Keywords: regression,
review
Whiteboard: r=axel@pike.org,sicking@bigbucks.com, Need sr=
You need to log in
before you can comment on or make changes to this bug.
Description
•