185498 - Don't rely on the ::CreateElement creates HTML elements

Status: VERIFIED FIXED

(Core :: XSLT, defect)

Description

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

txMozillaXMLOutput::startElement relies on that mDocument->CreateElement creates
an HTML element. This is only true if mDocument is an nsHTMLDocument, which
isn't always the case.

Unfortunatly i can't find any other way to create HTML elements. So the only way
of fixing this bug that i can think of is to check if mDocument is an
HTML-Document and if it isn't create a separate HTML-document
(mHTMLFactoryDocument) and use that to create the elements.

Thoughts?

Comment 1

[Axel Hecht]

I had that idea of keeping the element factories in our output handler for quite
some time, so if we use nsIDocument::GetAndIncrementContentID we should be
able to just use the right factory, set the ID on the content and leave the 
dom creation methods alone. Those are slow anyway, as we go thru the 
namespacemanager twice (once from ID to URI, and the document goes the same way
back)

Comment 2

[Axel Hecht]

Note that IMHO we should create XHTML for html output method and xml output
document. (That's different casing, and xhtml namespace ID instead of 0 for the
nodeinfo, IIRC.)

Comment 3

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

my vote goes towards sticking normal HTML-elements in the document even if it's
an XML document since mozilla is able to deal with that. Do what people ask for
even if it happens to be a bad thing.

peterv?

Comment 4

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

Attachment: patch to fix

This makes us create XHTML elements if the document we're transforming into
isn't HTML. This was the easiest way to go, and I don't really care too much
since this is only an issue for users of the deprecated js-interface.

I also fixed the fact that we were always wrapping table-contents in an
XHTML-<tbody>s. Now we'll create a normal HTML-<tbody> except when
::CreateElement won't give us what we need. (We still won't wrap tables in
xml-output though)

Comment 5

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

*** Bug 187977 has been marked as a duplicate of this bug. ***

Comment 6

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

-> me

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 114342 [details] [diff] [review]
patch to fix

>Index: source/xslt/txMozillaXMLOutput.cpp
>===================================================================

>     else {
>         mDocument = aResultDocument;
>         doc = do_QueryInterface(aResultDocument);
>+        
>+        nsCOMPtr<nsIHTMLDocument> htmlDoc = do_QueryInterface(aResultDocument);
>+        mDocumentIsHTML = !!htmlDoc;
>     }
>     mCurrentNode = mDocument;
>-
>+    

Don't add whitespace.

Comment 8

[Axel Hecht]

Comment on attachment 114342 [details] [diff] [review]
patch to fix

r=[e-mail]

Comment 9

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

Comment on attachment 114342 [details] [diff] [review]
patch to fix

I'd like to land this on the 1.3 branch if possible. The big interface rewrite
we did recently broke users of the old XSLT-js-interface so that they got XML
rather then HTML. This patch fixes this as well a small glitch where we insert
some XHTML <tbody>s in XSLT-generated HTML-pages.

We've got one bug filed because of this regression.

The patch is simple and should be safe thanks to that. It only affects pages
that uses XSLT.

Comment 10

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

this bug might be what is breaking P3P as well

Comment 11

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

checked in on trunk, keeping open for the 1.3 branch

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 114342 [details] [diff] [review]
patch to fix

a=asa (on behalf of drivers) for checkin to the 1.3 branch.

Comment 13

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

checked in on branch as well

Comment 14

[Axel Hecht]

mass verifying