146966 - Standalone Transformiix outputs HTML with XML prolog

Status: VERIFIED FIXED

(Core :: XSLT, defect, P3)

Description

[Peter Van der Beken [:peterv]]

If a stylesheet author doesn't set an xsl:output method and the stylesheet
produces HTML, we already started outputting XML (wrote out the prolog) by the
time we switch to HTML. I've made a new output handler that "caches" output
until we we switch to XML or HTML and then sends the cached output through the
right output handler.

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 2

[Peter Van der Beken [:peterv]]

Looking for reviews.

Comment 3

[Peter Van der Beken [:peterv]]

Attachment: v1.1

Comment 4

[Axel Hecht]

Comment on attachment 85652 [details] [diff] [review]
v1.1

>Index: extensions/transformiix/source/xslt/XSLTProcessor.cpp
>@@ -2310,22 +2330,36 @@
<...>
> #ifdef TX_EXE
>-            ostream* out;
>-            mOutputHandler->getOutputStream(&out);
>-            delete mOutputHandler;
>-            mOutputHandler = new txHTMLOutput();
>+        txUnknownHandler* oldHandler = (txUnknownHandler*)mOutputHandler;
>+        ostream* out;
>+        oldHandler->getOutputStream(&out);
move getting the old handler down to ...
>+#endif
>+        if (format->mMethod == eMethodNotSet) {
>+            // XXX Should check for whitespace-only sibling text nodes
>+            if ((aNsID == kNameSpaceID_None) &&
>+                aName.isEqualIgnoreCase("html")) {
>+                // Switch to html output mode according to the XSLT spec.
>+                format->mMethod = eHTMLOutput;
>+#ifndef TX_EXE
>+                mOutputHandler->setOutputFormat(format);
>+#endif
>+            }
>+#ifdef TX_EXE
... where it's needed? 
>+            if (format->mMethod == eHTMLOutput) {
>+                mOutputHandler = new txHTMLOutput();
>+            }
>+            else {
>+                mOutputHandler = new txXMLOutput();
>+                format->mMethod = eXMLOutput;
>+            }
>             NS_ASSERTION(mOutputHandler, "Setting mResultHandler to NULL!");
>             mOutputHandler->setOutputStream(out);
>+            mOutputHandler->setOutputFormat(format);
>             mResultHandler = mOutputHandler;
>+            oldHandler->flush(mOutputHandler);
>+            delete oldHandler;
> #endif
>-            mOutputHandler->setOutputFormat(format);
>         }
>         mHaveDocumentElement = MB_TRUE;
>     }

>Index: extensions/transformiix/source/xslt/txUnknownHandler.cpp
<...>
>+void txUnknownHandler::processingInstruction(const String& aTarget,
>+                                             const String& aData)
>+{
<...>
>+    transaction->mStringOne = aData;
>+    transaction->mStringTwo = aData;
not really, right?
>+    addTransaction(transaction);
>+}
>+

>+void txUnknownHandler::addTransaction(txOutputTransaction* aTransaction)
>+{
>+    if (mTotal == mMax) {
>+        //cout << "You should consider setting an output method!" << endl;
remove it or make it an assertion

>Index: extensions/transformiix/source/xslt/txUnknownHandler.h
>===================================================================
<...>
>+
>+#define kReasonableTransactions 8
make this a static member of txUnknownHandler.
>+
>+struct txOutputTransaction
>+{
>+public:
>+    enum txTransactionType {
>+        eCharacterTransaction,
>+        eCharacterNoOETransaction,
>+        eCommentTransaction,
>+        eEndDocumentTransaction,
>+        ePITransaction,
>+        eStartDocumentTransaction
>+    };
>+    txTransactionType mType;
>+};
>+
>+struct txOneStringTransaction : public txOutputTransaction
>+{
>+    String mString;
>+};
>+
>+struct txTwoStringTransaction : public txOutputTransaction
>+{
>+    String mStringOne;
>+    String mStringTwo;
>+};
>+

I don't like the |struct|s, make that |class|es. I'd say give them
constructors,
too, that'll ease the code in txUnknownHandler quite a bit.

Comment 5

[Peter Van der Beken [:peterv]]

Attachment: v1.2

Addressed comments.

Comment 6

[Axel Hecht]

please add a virtual destructor to txOutputTransaction, so that the destructor
of txUnknownHandler frees the string members, too, if 
txUnknownHandler::flush wasn't called.

Comment 7

[Peter Van der Beken [:peterv]]

Attachment: v1.3

Comment 8

[Axel Hecht]

Comment on attachment 87052 [details] [diff] [review]
v1.3

in XSLTProcessor.cpp:2106, you might wanna set the output method to eXMLOutput,
and add a comment that this is only triggered for result docs without
documentElement.
This is really a border case, I don't care for which you go, so
r=[e-mail]

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 87052 [details] [diff] [review]
v1.3

sr=jst

Comment 10

[Peter Van der Beken [:peterv]]

Checked in.

Comment 11

[Axel Hecht]

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