Closed Bug 73936 Opened 24 years ago Closed 24 years ago

xsl:include/xsl:import/document() doesn't work

Categories

(Core :: XSLT, defect, P1)

Product:
Core
Component:
XSLT
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.4

People

(Reporter: markushuebner, Assigned: peterv)

References

(
URL
)

Details

(Whiteboard: ns6.1)

Attachments

(6 files)

Fix for syncloader
13.55 KB, patch
Details | Diff | Splinter Review
Fixes all known issues with document loading in Transformiix
35.60 KB, patch
Details | Diff | Splinter Review
patch with the fixes for "" and event listener leak
35.81 KB, patch
Details | Diff | Splinter Review
Hopefully final patch
41.79 KB, patch
Details | Diff | Splinter Review
Adressed some reviewer comments
39.67 KB, patch
Details | Diff | Splinter Review
And another revision
41.91 KB, patch
Details | Diff | Splinter Review
no display of content elements - works fine in MSIE :-/
Severity: normal → blocker
Severity: blocker → normal
OS: Windows 2000 → All
Hardware: PC → All
Also seeing this with 2001032804 / Win98SE.
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
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 → ---
WORKSFORME Platform: PC OS: Windows 98 Mozilla build: 2001040404 Download the latest nightly and see if that fixes the problem.
'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
so the external stylesheet isn't resolved correctly ...
Severity: normal → major
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
Target Milestone: mozilla0.9.1 → mozilla1.1
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
changing summary
Summary: xsl:include doesn't work → xsl:include/xsl:import/document() doesn't work
*** Bug 81124 has been marked as a duplicate of this bug. ***
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.
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.
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
This patch should include the renaming of nsLoadListenerProxy to txLoadListenerProxy, IMHO. Axel
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
Blocks: 68824
Blocks: 78068
Keywords: nsBranch
Target Milestone: mozilla0.9.2 → mozilla0.9.3
nsBranch ok, limited to XSLT only.
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=
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?
Keywords: nsBranch
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
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)
Don't you need changes to nsXULDocument to prevent it from having pure virtual methods?
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.
And i tested it with MFCEmbed, WinEmbed and PPEmbed.
Target Milestone: mozilla0.9.3 → mozilla0.9.4
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
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=
sr=jst
Aaaaaaaaah.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Unfortunately the testcase at http://www.world-direct.com/gia/test/mozillatest.asp still doesn't display correctly.
tried again with a later build - 2001080303 and everything is working fine now :)
verified, per comments of reporter and myself
Status: RESOLVED → VERIFIED
Keywords: regression, review
Whiteboard: r=axel@pike.org,sicking@bigbucks.com, Need sr=
adding ns6.1, this bug didn't make it to that release.
Whiteboard: ns6.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: