Standalone Transformiix outputs HTML with XML prolog

VERIFIED FIXED in mozilla1.1beta

Status

()

Core
XSLT
P3
normal
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: peterv, Assigned: peterv)

Tracking

Trunk
mozilla1.1beta
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

16 years ago
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.
(Assignee)

Comment 1

16 years ago
Created attachment 85014 [details] [diff] [review]
v1
(Assignee)

Comment 2

16 years ago
Looking for reviews.
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.1alpha
(Assignee)

Comment 3

16 years ago
Created attachment 85652 [details] [diff] [review]
v1.1
Attachment #85014 - Attachment is obsolete: true

Comment 4

16 years ago
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.
(Assignee)

Comment 5

16 years ago
Created attachment 85836 [details] [diff] [review]
v1.2

Addressed comments.
Attachment #85652 - Attachment is obsolete: true

Comment 6

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

Updated

16 years ago
Target Milestone: mozilla1.1alpha → mozilla1.1beta
(Assignee)

Comment 7

16 years ago
Created attachment 87052 [details] [diff] [review]
v1.3
Attachment #85836 - Attachment is obsolete: true

Comment 8

16 years ago
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=axel@pike.org
Attachment #87052 - Flags: review+
Comment on attachment 87052 [details] [diff] [review]
v1.3

sr=jst
Attachment #87052 - Flags: superreview+
(Assignee)

Comment 10

16 years ago
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 11

16 years ago
we didn't verify for a long time.
I really checked, so VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.