56087 - XSLTProcessor::TransformDocument can't work on existing document

Status: VERIFIED FIXED

(Core :: XSLT, defect, P3)

Description

[Axel Hecht]

Hi,
I try to get an UI working again for transformiix.
(Curious? http://www.numerik.uni-kiel.de/~ah/mozilla/newUI.tar.gz)
This is supposed to do multiple transformations on documents, so you will 
(as in most cases, I guess) end up with calling the transformer with an 
already existing document.
As the result document is having a document node, we try to add a 
transformiix:result node, which won't work for XHTML output.
Segfault.
We need to clear the resultDocument in TransformDocument before populating it 
with the transformed content.
So that's about it.
Peter, you know most about layout, so assigned to you ;-).

Axel

Comment 1

[Peter Van der Beken [:peterv]]

Have a patch. Will attach next week.

Comment 2

[Peter Van der Beken [:peterv]]

Netscape.com work.

Comment 3

[Dan Rosen]

peter: is there anything i can help with here? i know very little about layout,
but i can try...

Comment 4

[Axel Hecht]

We should call Reset(nsnull,nsnull); on the incoming result document before
XSLTProcessor.cpp:1742
to clear the incoming document.
The downside is, that function isn't part of a interface, so we had to add it.
(IIRC, that is)

Reset is implemented by nsDocument, nsXMLDocument and nsHTMLDocument, but not in
nsXULDocument.

Which would be the right interface?

Axel

Comment 5

[Dan Rosen]

It's quite unfortunate that we have three separate object models, since you want
to be able to transform existing XUL documents as well as any other type. I'm
thinking you want to use the XML document API...

Comment 6

[Johnny Stenback (:jst)]

Sounds like we'd need to make the Reset method available in an interface, I'd
say we could add it to nsIDocument, all our documents implement nsIDocument.
Would that solve this problem?

Comment 7

[Peter Van der Beken [:peterv]]

Moving forward.

Comment 8

[Peter Van der Beken [:peterv]]

Attachment: Call Reset on the output document

Comment 9

[Peter Van der Beken [:peterv]]

The attached patch gets the source document's loadgroup, creates a temporary
channel with the source document's URL and calls Reset on the output document
with those two objects. This clears the output document and causes the output
document to pick up many of the characteristics of the source document (which is
a good thing and necessary for things like scripts to work). I need a review and
a super-review.

Comment 10

[Dan Rosen]

Peter: looks pretty straightforward to me. Just a couple things:

  - Just to double-check, do you want to fail silently? That is, your
    if(whatever) checks don't have an else returning an error -- maybe instead
    of if(whatever), you'd rather use NS_ENSURE_TRUE and NS_ENSURE_SUCCESS.
    Maybe. Or maybe not. :)

  - Also, i don't understand why the change to nsISupports, but that might
    just be me being dumb.

Either way, r=dr.

Comment 11

[Peter Van der Beken [:peterv]]

1) Reset handles nsnull for the loadgroup and channel just fine. We shouldn't
bail out if we don't have the loadgroup or channel, we want to transform in that
case too, and the Reset will correctly empty the document then.
2) We don't need to cast to nsIDOMNode, the Notify call further down in the code
takes an nsISupports.

Comment 12

[Johnny Stenback (:jst)]

sr=jst

Comment 13

[Peter Van der Beken [:peterv]]

Checked in.

Comment 14

[Axel Hecht]

I get a segfault when trying to transform into the document of a xul:iframe.

Here is the stack trace:
#0  0xfbd706ac in ?? ()
    at
/tmp/mozilla/layout/svg/base/src/../../../base/public/nsIStyleContext.h:165 from
/tmp/build/dist/bin/components/libgklayout.so
#1  0xfba9cc74 in nsCSSFrameConstructor::ProcessChildren (this=0x9e4c78, 
    aPresShell=0x9e4708, aPresContext=0xa29fb8, aState=@0xffbeb748, 
    aContent=0xa8bc50, aFrame=0xa56b68, aCanHaveGeneratedContent=1, 
    aFrameItems=@0xffbeb560, aParentIsBlock=1, aTableCreator=0x0)
    at /tmp/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:11223
#2  0xfba7dc34 in nsCSSFrameConstructor::ConstructDocElementFrame (
    this=0x9e4c78, aPresShell=0x9e4708, aPresContext=0xa29fb8, 
    aState=@0xffbeb748, aDocElement=0xa8bc50, aParentFrame=0xa560bc, 
    aParentStyleContext=0xa5ee50, aNewFrame=@0xffbeb81c)
    at /tmp/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3533
#3  0xfba90700 in nsCSSFrameConstructor::ContentInserted (this=0x9e4c78, 
    aPresContext=0xa29fb8, aContainer=0x0, aChild=0xa8bc50, 
    aIndexInContainer=0, aFrameState=0x0)
    at /tmp/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:8401
#4  0xfc93f1c8 in StyleSetImpl::ContentInserted (this=0x958f48, 
    aPresContext=0xa29fb8, aContainer=0x0, aChild=0xa8bc50, 
    aIndexInContainer=0) at /tmp/mozilla/content/base/src/nsStyleSet.cpp:1249
#5  0xfb9e1030 in PresShell::ContentInserted (this=0x9e4708, 
    aDocument=0x7edda8, aContainer=0x0, aChild=0xa8bc50, aIndexInContainer=0)
    at /tmp/mozilla/layout/html/base/src/nsPresShell.cpp:4575
#6  0xfc8ae818 in nsDocument::ContentInserted (this=0x7edda8, aContainer=0x0, 
    aChild=0xa8bc50, aIndexInContainer=0)
    at /tmp/mozilla/content/base/src/nsDocument.cpp:1554
#7  0xfc8b3e6c in nsDocument::InsertBefore (this=0x7edda8, aNewChild=0xa8bc7c, 
    aRefChild=0x0, aReturn=0xffbebca0)
    at /tmp/mozilla/content/base/src/nsDocument.cpp:2600
#8  0xfc8b4824 in nsDocument::AppendChild (this=0x7edda8, aNewChild=0xa8bc7c, 

I did the following:

ddd = document.implementation.createDocument("","",null);
ddd.load('file:///home/ah/projekte/mozilla/xslt/conf/axes/axes01.xml');
sty = document.implementation.createDocument("","",null);
sty.load('file:///home/ah/projekte/mozilla/xslt/conf/axes/axes01.xsl');
mfr = document.createElement('iframe');mfr.setAttribute('flex','1')
document.firstChild.appendChild(mfr);
mfr.setAttribute('src','file:///home/ah/projekte/mozilla/xslt/conf/axes/axes01.xml');
res = mfr.docShell.contentViewer.DOMDocument;
xslt = new XSLTProcessor();
xslt.transformDocument(ddd,sty,res,null);

All documents were loaded successfully (this is from a interactive xul js shell)

Axel

Comment 15

[Peter Van der Beken [:peterv]]

Attachment: Input document

Comment 16

[Peter Van der Beken [:peterv]]

Attachment: XSLT stylesheet

Comment 17

[Peter Van der Beken [:peterv]]

Attachment: Testcase

This testcase loads the input document and the XSLT stylesheet and transforms
them directly into the iframe's document.

Comment 18

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 19

[Axel Hecht]

that thing doesn't build out of the box, I had to add the includes for both
the nsIHTMLStyleSheet and nsIHTMLCSSStyleSheet.
I wonder if the stuff should go into nsDocument instead of having duplicated
code in both docs. Not sure if that's hell burning impact patch though.
If you don't share the code here, make sure that there's a comment in bugzilla
that you added code that should be shared.

So, after building, I tried the testcase, and it doesn't really work :-(.

Problems:

1) it doesn't display stuff, at least for my tree. I get a hell of assertions,


Document http://bugzilla.mozilla.org/attachment.cgi?id=78340&action=view loaded
successfully
frame: Inline(hr)(4) (8b9130) style: 8b90a8 :before {}
Wrong parent style context:  style: 8b8f8c {}
should be using:  style: 8b86cc {}
Style contexts are not in the same style context tree.

frame: Inline(hr)(4) (8b92e8) style: 8b9260 :after {}
Wrong parent style context:  style: 8b8f8c {}
should be using:  style: 8b86cc {}
Style contexts are not in the same style context tree.

frame: Inline(hr)(4) (8b92e8) style: 8b9260 :before {}
Wrong parent style context:  style: 8b8e64 {}
should be using:  style: 9cfc04 {}
Style contexts are not in the same style context tree.

frame: Inline(hr)(4) (8b9130) style: 8b8770 :after {}
Wrong parent style context:  style: 8b8e64 {}
should be using:  style: 9cfc04 {}
Style contexts are not in the same style context tree.

Once I'm there and see the iframe that lost it's "file not found" page, I hit
back and forth, and see what's coming next:

Document http://bugzilla.mozilla.org/show_bug.cgi?id=56087 loaded successfully
WARNING: NS_ENSURE_TRUE(tmp) failed, file
/tmp/mozilla/content/html/content/src/nsHTMLIFrameElement.cpp, line 213
Document http://bugzilla.mozilla.org/attachment.cgi?id=78340&action=view loaded
successfully
WARNING: NS_ENSURE_TRUE(aOutputDoc) failed, file
/tmp/mozilla/extensions/transformiix/source/xslt/XSLTProcessor.cpp, line 2336
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80070057
(NS_ERROR_ILLEGAL_VALUE) [nsIDocumentTransformer.transformDocument]"  nsresult:
"0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
http://bugzilla.mozilla.org/attachment.cgi?id=78340&action=view ::
tryToTransform :: line 42"  data: no]
************************************************************

Comment 20

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I just realized that my work on bug 130161 might invalidat this bug. The 
nsIDocumentTransformer interface is modified so that it always returns the 
result document, and the interface we've had in mind for JS only uses the 
result document for creating a result doc-fragment, the document itself isn't 
modified.

Comment 21

[Peter Van der Beken [:peterv]]

Attachment: v2

Includes a patch for the P3P policy viewer to transform directly into the
result document.

Comment 22

[Peter Van der Beken [:peterv]]

Attachment: Stylesheet cleanup for ns*Document

Comment 23

[Peter Van der Beken [:peterv]]

Needed for P3P policy viewer so nominating for nsbeta1

Comment 24

[Peter Van der Beken [:peterv]]

BTW, I split out the cleanup that I originaly did as part of this fix since the
new fix doesn't touch those files anymore. I'd like attachment 79250 [details] [diff] [review] on the
trunk but I don't think we want to check it in on the branch.

Comment 25

[harishd]

Comment on attachment 79249 [details] [diff] [review]
v2

r=harishd

Comment 26

[Axel Hecht]

applied and tested attachement 79249

back and forward still put me in assertion hell, and bring up a js error:

WARNING: NS_ENSURE_TRUE(tmp) failed, file
/tmp/mozilla/content/html/content/src/nsHTMLIFrameElement.cpp, line 213
Document http://bugzilla.mozilla.org/attachment.cgi?id=78340&action=view loaded
successfully
WARNING: NS_ENSURE_TRUE(aOutputDoc) failed, file
/tmp/mozilla/extensions/transformiix/build/../source/xslt/XSLTProcessor.cpp,
line 2336
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80070057
(NS_ERROR_ILLEGAL_VALUE) [nsIDocumentTransformer.transformDocument]"  nsresult:
"0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
http://bugzilla.mozilla.org/attachment.cgi?id=78340&action=view ::
tryToTransform :: line 42"  data: no]
************************************************************
###!!! ASSERTION: different documents: 'oldAncestor == newAncestor', file
/tmp/mozilla/content/events/src/nsEventStateManager.cpp, line 3474
###!!! Break: at file /tmp/mozilla/content/events/src/nsEventStateManager.cpp,
line 3474

note: venkman says that output is null. That happens as well if I run this 
locally, but I have to remove the src attribute for that to work at least on
first load. 

jst claims that his iframe code should fix this, could someone verify?

I don't see the context branch being in the way for landing this.

Comment 27

[Peter Van der Beken [:peterv]]

I see none of those assertions. Here's my output:

Document http://bugzilla.mozilla.org/show_bug.cgi?id=56087 loaded successfully
###!!! ASSERTION: not a file URL: 'Error', file nsStandardURL.cpp, line 2147
WEBSHELL+ = 4
###!!! ASSERTION: not a file URL: 'Error', file nsStandardURL.cpp, line 2147
###!!! ASSERTION: not a file URL: 'Error', file nsStandardURL.cpp, line 2147
###!!! ASSERTION: not a file URL: 'Error', file nsStandardURL.cpp, line 2147
###!!! ASSERTION: not a file URL: 'Error', file nsStandardURL.cpp, line 2147
GetPrimaryFrameFor() called while FrameManager is being destroyed!
GetPrimaryFrameFor() called while FrameManager is being destroyed!
GetPrimaryFrameFor() called while FrameManager is being destroyed!
GetPrimaryFrameFor() called while FrameManager is being destroyed!
Document http://bugzilla.mozilla.org/attachment.cgi?id=78340&action=view loaded
successfully
•••WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsDOMClassInfo.cpp,
line 3608
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsDOMClassInfo.cpp, line 3608
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'Permission denied to get property ChromeWindow.XMLDocument' when
calling method: [nsIDOMEventListener::handleEvent]"  nsresult: "0x8057001e
(NS_ERROR_XPC_JS_THREW_STRING)"  location: "<unknown>"  data: no]
************************************************************
•••WARNING: not supported for views, file nsScrollPortView.cpp, line 113
WARNING: not supported for views, file nsScrollPortView.cpp, line 113
•••WARNING: not supported for views, file nsScrollPortView.cpp, line 113
WARNING: not supported for views, file nsScrollPortView.cpp, line 113
•••WARNING: not supported for views, file nsScrollPortView.cpp, line 113
WARNING: not supported for views, file nsScrollPortView.cpp, line 113
Document http://bugzilla.mozilla.org/show_bug.cgi?id=56087 loaded successfully
WEBSHELL- = 3

Comment 28

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 79249 [details] [diff] [review]
v2

Please do not attach diffs from the ns tree to Mozilla bugs; open separate
bugscape bugs for those parts.

Since we no longer need p3pDummy.xml, please cvs remove that as part of this
patch.

sr=heikki

Comment 29

[scottputterman]

Please check this into the trunk and update this bug after testing this.

Comment 30

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Fixed 04/16/2002 02:26 on trunk.

Comment 31

[scottputterman]

Please test this on the trunk and update the bug with the results.

What is the actual bug that the user sees in terms of the P3P policy viewer?

Comment 32

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This blocks the "no scrollbars in generated P3P summary" bug.

Comment 33

[Peter Van der Beken [:peterv]]

This is preparation for fixing bug 135825 (No scrollbars in P3P summary window).
I can't fix that bug without this fix. The most important thing to test is that
this didn't regress the Policy Viewer. I tested it and haven't seen any
problems, but someone should confirm that. Also, attachment 78340 [details] should open
without crashing and the transform should succeed ("Text coming from the XSLT
stylesheet" should be displayed in the iframe).

Comment 34

[John Unruh]

Tested on 4/17 Linux trunk build. I don't see any regressions in the P3P policy 
summary viewer.

Comment 35

[scottputterman]

adding adt1.0.0+ on behalf of the adt.  Please check into the branch as soon as
possible and add the fixed1.0.0 keyword.

Comment 36

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Sent email for drivers approval.

Comment 37

[Peter Van der Beken [:peterv]]

Still awaiting drivers approval.

Comment 38

[Scott Collins]

Comment on attachment 79249 [details] [diff] [review]
v2

a=scc for checkin to the mozilla 1.0 branch

Comment 39

[scottputterman]

Now that you have Mozilla approval, please check in as soon as possible!

Comment 40

[Peter Van der Beken [:peterv]]

Checked in on the 1.0 branch.

Comment 41

[Axel Hecht]

we didn't verify for a long time.
I really checked, so VERIFIED.