- Bug fixes to new fixup scheme

8 files changed, 175 insertions, 104 deletions

diff --git a/src/domhelpers.cpp b/src/domhelpers.cpp
index ac93f10..7ecf963 100644
--- a/src/domhelpers.cpp
+++ b/src/domhelpers.cpp
@@ -186,6 +186,8 @@ DOM::Element DOMHelpers::getChildElement(const DOM::Node& parent, const string&
     {
         if(isElement(child, name))
             return (DOM::Element&)child;
+
+		child = child.getNextSibling();
     }
 
     return DOM::Element();
@@ -226,6 +228,8 @@ void ElementTable::load(const DOM::Node& parent, const string& name)
             if(!id.empty())
                 insert(make_pair(id, el));
         }
+
+		child = child.getNextSibling();
     }
 }
 
@@ -250,30 +254,55 @@ void ElementTable::removeIds()
 
 void ElementIterator::next()
 {
-    if(m_current == NULL)
-        return;
+	if(m_flags == AFTER_LAST)
+		return;
+
+	if(m_flags == BEFORE_FIRST)
+		m_current = m_top;
+
+	ASSERT(m_current != NULL);
 
     DOM::Node n;
 
     // Always descend into children first
     if(m_current.hasChildNodes())
     {
-        m_current = nextel(m_current.getFirstChild());
-        if(m_current != NULL)
+        n = nextel(m_current.getFirstChild());
+        if(n != NULL)
+		{
+			m_current = n;
+			m_flags = ITERATING;
             return;
+		}
     }
 
     // Look for siblings along the current level
-    m_current = nextel(m_current.getNextSibling());
-    if(m_current != NULL)
+    n = nextel(m_current.getNextSibling());
+    if(n != NULL)
+	{
+		m_current = n;
+		m_flags = ITERATING;
         return;
-
-    // Go back up to parent
-    m_current = m_current.getParentNode();
-
-    // But check top against parent
-    if(m_current == NULL || m_current == m_top)
-        m_current = NULL;
+	}
+
+	for(;;)
+	{
+	    // Go back up to parent, and get it's next sibling
+		m_current = m_current.getParentNode();
+		if(m_current == NULL || m_current == m_top)
+			break;
+
+		n = nextel(m_current.getNextSibling());
+		if(n != NULL)
+		{
+			m_current = n;
+			m_flags = ITERATING;
+			return;
+		}
+	}
+
+	m_flags = AFTER_LAST;
+    m_current = NULL;
 }
 
 DOM::Element ElementIterator::nextel(DOM::Node node)
@@ -291,29 +320,41 @@ DOM::Element ElementIterator::nextel(DOM::Node node)
 
 void ElementIterator::prev()
 {
-    /* Allow backing into the iterator */
-    if(m_current == NULL)
-        m_current = m_top;
+	if(m_flags == BEFORE_FIRST)
+		return;
 
     DOM::Node n;
 
-    // Always descend into children first
-    if(m_current.hasChildNodes())
-    {
-        m_current = prevel(m_current.getLastChild());
-        if(m_current != NULL)
-            return;
-    }
+	if(m_flags == AFTER_LAST)
+        n = m_top;
+	else
+		n = prevel(m_current.getPreviousSibling());
+
+	while(n != NULL)
+	{
+		m_current = n;
+
+		n = n.hasChildNodes() ?
+				prevel(m_current.getLastChild()) : DOM::Node();
+
+		if(n == NULL)
+		{
+			m_flags = ITERATING;
+			return;
+		}
+	}
+
+	// Go back up to parent
+	m_current = m_current.getParentNode();
+
+	if(m_current == NULL || m_current == m_top)
+	{
+		m_flags = BEFORE_FIRST;
+		m_current = NULL;
+		return;
+	}
 
-    // Look for siblings along the current level
-    m_current = prevel(m_current.getPreviousSibling());
-    if(m_current != NULL)
-        return;
 
-    // Go back up to parent
-    DOM::Node parent = m_current.getParentNode();
-    if(parent != m_top)
-        m_current = (DOM::Element&)parent;
 }
 
 
diff --git a/src/domhelpers.h b/src/domhelpers.h
index d125f80..fd386f8 100644
--- a/src/domhelpers.h
+++ b/src/domhelpers.h
@@ -113,11 +113,11 @@ class ElementIterator
 {
 public:
     ElementIterator()
-        { m_current = NULL; }
+        { m_flags = AFTER_LAST; }
     ElementIterator(const DOM::Element& top)
-        { m_top = top; m_current = top; next(); }
+        { m_top = top; m_flags = BEFORE_FIRST; next(); }
     ElementIterator(const ElementIterator& x)
-        { m_top = x.m_top; m_current = x.m_current; }
+        { m_top = x.m_top; m_current = x.m_current; m_flags = x.m_flags; }
 
     const DOM::Element& operator*() const
         { return m_current; }
@@ -144,14 +144,21 @@ protected:
 // Data
 protected:
 
+	enum
+	{
+		ITERATING,
+		BEFORE_FIRST,
+		AFTER_LAST
+	};
+
     DOM::Element m_top;
     DOM::Element m_current;
-    bool m_done;
+	int m_flags;
 };
 
 // friend functions
 inline bool operator==(const ElementIterator& x, const ElementIterator& y)
-   { return y.m_current == x.m_current && y.m_top == x.m_top; }
+   { return y.m_current == x.m_current && y.m_flags == x.m_flags; }
 inline bool operator!=(const ElementIterator& x, const ElementIterator& y)
    { return (!(x == y)); }
 
diff --git a/src/tags.h b/src/tags.h
index 24af8b8..cd7b211 100644
--- a/src/tags.h
+++ b/src/tags.h
@@ -49,6 +49,7 @@ static const char* kAtCell = "i_cell";
 static const char* kElListtable = "i_listtable";
 static const char* kElListdef = "i_listdef";
 static const char* kElFontTable = "i_fonttable";
+static const char* kElFontDef = "i_fontdef";
 
 // All the main tags
 static const char* kElPara = "para";
diff --git a/src/usuals.h b/src/usuals.h
index c9843b0..eeb13b1 100644
--- a/src/usuals.h
+++ b/src/usuals.h
@@ -43,6 +43,11 @@
 #endif
 
 #ifdef _WIN32
+/* Suppress stupid errors */
+#pragma warning( disable : 4786 )
+#endif
+
+#ifdef _WIN32
 #include "config.win32.h"
 #else
 #include "config.h"
diff --git a/src/xmlcomposer.cpp b/src/xmlcomposer.cpp
index 2b65833..398cb55 100644
--- a/src/xmlcomposer.cpp
+++ b/src/xmlcomposer.cpp
@@ -220,13 +220,25 @@ void XmlComposer::pushElement(const DOM::Element& element)
 	m_curLevel->setElement(element);
 }
 
+void XmlComposer::pushTopElement(const DOM::Element& element)
+{
+	ASSERT(m_curLevel != NULL);
+	DOM::Element top = m_document.getDocumentElement();
+	if(top.hasChildNodes())
+		top.insertBefore(element, top.getFirstChild());
+	else
+		top.appendChild(element);
+
+	m_curLevel->setElement(element);
+}
+
 DOM::Element XmlComposer::popElement()
 {
 	DOM::Element element = getElement();
 	ASSERT(m_curLevel != NULL);
 
 	DOM::Node parent = element.getParentNode();
-	ASSERT(parent.getNodeType() == DOM::Node::ELEMENT_NODE);
+	ASSERT(parent != NULL && parent.getNodeType() == DOM::Node::ELEMENT_NODE);
 
 	// Set it deep so it replaces the current element
 	m_curLevel->setElement((DOM::Element&)parent, true);
@@ -317,19 +329,21 @@ void XmlComposer::incrementAutoCount(int type)
  */
 
 #define AN_ELEMENT(name) \
- m_composer->pushElement(m_composer->createElement(name))
+	m_composer->pushElement(m_composer->createElement(name))
+#define AN_TOP_ELEMENT(name) \
+	m_composer->pushTopElement(m_composer->createElement(name))
 #define AN_POP_ELEMENT() \
     m_composer->popElement()
 #define AN_ATTRIBUTE(name, value) \
-   m_composer->setAttribute(name, value)
+	m_composer->setAttribute(name, value)
 #define AN_DESTINATION_ATTR(name) \
-  m_composer->setDestination(new Attribute(name))
+	m_composer->setDestination(new Attribute(name))
 #define AN_DESTINATION(cls) \
-  m_composer->setDestination(new cls)
+	m_composer->setDestination(new cls)
 #define AN_ANALYSER(cls) \
- m_composer->setAnalyser(AnalyserPtr(new cls))
+	m_composer->setAnalyser(AnalyserPtr(new cls))
 #define AN_SET_ANALYSER(cls) \
-   m_composer->setAnalyser(AnalyserPtr(cls))
+	m_composer->setAnalyser(AnalyserPtr(cls))
 #define HAS_PARAM (flags & kHasParam)
 #define DEFAULT_CONTROLWORD processDefault(cw, flags, param)
 #define DUMMY  1 == 1
@@ -394,7 +408,6 @@ DOM::Element XmlComposer::BaseAnalyser::getCurrentBlock()
 		node = node.getLastChild();
 
 	return DOMHelpers::getPriorElement(node, kElBlock);
-
 }
 
 bool XmlComposer::BaseAnalyser::processTextContent(const string& cw, int flags, int param)
@@ -617,7 +630,7 @@ ON_GROUPEND(Upr)
 
 ON_INITIALIZE(Stylesheet)
 {
-	AN_ELEMENT(kElStylesheet);
+	AN_TOP_ELEMENT(kElStylesheet);
 }
 
 ON_GROUPSTART(Stylesheet)
@@ -705,7 +718,7 @@ ON_GROUPEND(Style)
 
 ON_INITIALIZE(FontTable)
 {
-    AN_ELEMENT(kElFontTable);
+	AN_TOP_ELEMENT(kElFontTable);
     AN_DESTINATION(Null);
 }
 
@@ -720,7 +733,7 @@ ON_GROUPSTART(FontTable)
 
 ON_INITIALIZE(Font)
 {
-   AN_ELEMENT(kElFont);
+   AN_ELEMENT(kElFontDef);
    AN_DESTINATION_ATTR(kAtName);
 }
 
@@ -744,7 +757,7 @@ ON_GROUPSTART(Font)
 
 ON_INITIALIZE(ListTable)
 {
-	AN_ELEMENT(kElListtable);
+	AN_TOP_ELEMENT(kElListtable);
 }
 
 ON_GROUPSTART(ListTable)
@@ -763,7 +776,10 @@ ON_INITIALIZE(List)
 {
     // Create a default element
   	AN_ELEMENT(kElListdef);
-	AN_ATTRIBUTE(kAtType, kValDisc);
+
+	if(DO_EXTRAS())
+		AN_ATTRIBUTE(kAtType, kValDisc);
+
 	AN_ATTRIBUTE(kAtOrdered, L"0");
 	levelsSeen = 0;
 }
@@ -913,7 +929,7 @@ ON_CONTROLWORD(ListOverrideTable)
 			if(curList != NULL)
 			{
 				parent.appendChild(curList);
-				curList.setAttribute(kAtList, NUM_ATTR(lsId));
+				curList.setAttribute(kAtId, NUM_ATTR(lsId));
 			}
 		}
 
@@ -928,7 +944,7 @@ ON_CONTROLWORD(ListOverrideTable)
 ON_INITIALIZE(Info)
 {
 	// Create a new element
-	AN_ELEMENT(kElInfo);
+	AN_TOP_ELEMENT(kElInfo);
 	AN_DESTINATION(Null);
 }
 
@@ -1017,8 +1033,6 @@ ON_INITIALIZE(Content)
 
 ON_CHARDATA(Content)
 {
-    int v;
-
 	// Create the first time we get content
 	if(!created)
 	{
diff --git a/src/xmlcomposer.h b/src/xmlcomposer.h
index e73c032..83ea4cd 100644
--- a/src/xmlcomposer.h
+++ b/src/xmlcomposer.h
@@ -81,6 +81,9 @@ public:
     // Push an XML element on the current level
 	void pushElement(const DOM::Element& element);
 
+	// Push an element at the top of the document
+	void pushTopElement(const DOM::Element& element);
+
     // Replace current XML element with given element
 	void replaceElement(const DOM::Element& element);
 
diff --git a/src/xmlfixups.cpp b/src/xmlfixups.cpp
index b8c84f4..10b35f1 100644
--- a/src/xmlfixups.cpp
+++ b/src/xmlfixups.cpp
@@ -42,7 +42,7 @@
 #include "tags.h"
 
 static const char* kNoDuplicates[] =
-    { kElB, kElU, kElI, kElColor, kElHide, kElColor, kElSuper, kElSub, NULL };
+    { kElB, kElU, kElI, kElColor, kElHide, kElSuper, kElSub, NULL };
 
 static const char* kRemoveTags[] =
  { kElDest, kElListdef, kElListtable, kElFontTable, NULL };
@@ -62,7 +62,7 @@ static const char* kConsolidateStart[] =
 void loadStringSet(StringSet& set, const char** strings)
 {
     while(*strings)
-        set.insert(string(*strings));
+        set.insert(string(*(strings++)));
 }
 
 XmlFixups::XmlFixups()
@@ -412,54 +412,14 @@ void XmlFixups::runPassTwo(const DOM::Document& doc)
 
     DOM::Element top = doc.getDocumentElement();
 
-    // Get stylesheet block
-    DOM::Element el = DOMHelpers::getChildElement(top, kElStylesheet);
-    if(el != NULL)
-    {
-        // Load the styles into a id mapped table
-        styles.load(el, kElStyle);
-
-        if(!styles.empty())
-        {
-            styles.removeIds();
-            haveStyles = true;
-        }
-    }
-
-    // Get the font block
-    el = DOMHelpers::getChildElement(top, kElFontTable);
-    if(el != NULL)
-    {
-        // Load the fonts into an id mapped table
-        fonts.load(el, kElFont);
-
-        if(!fonts.empty())
-        {
-            fonts.removeIds();
-            haveFonts = true;
-        }
-    }
-
-    // Get the list definition block
-    el = DOMHelpers::getChildElement(top, kElListtable);
-    if(el != NULL)
-    {
-        // Load the lists into an id mapped table
-        lists.load(el, kElListdef);
-
-        if(!lists.empty())
-        {
-            lists.removeIds();
-            haveLists = true;
-        }
-    }
-
     NodeStack toStart;      // Nodes that get moved to beginning of document
     NodeStack toEnd;        // Nodes that get moved to the end of the document
 
     ElementIterator it(top);
     ElementIterator end;
 
+	DOM::Element el;
+
     for( ; it != end; ++it)
     {
         el = *it;
@@ -472,7 +432,46 @@ void XmlFixups::runPassTwo(const DOM::Document& doc)
         el.setUserData((void*)PASS_TWO);
         string name = el.getNodeName();
 
-        if(name == kElBlock)
+		// Get stylesheet block
+		if(name == kElStylesheet)
+		{
+	        // Load the styles into a id mapped table
+		    styles.load(el, kElStyle);
+
+			if(!styles.empty())
+			{
+				styles.removeIds();
+				haveStyles = true;
+			}
+		}
+
+		// The Font Table
+	    else if(name == kElFontTable)
+		{
+			// Load the fonts into an id mapped table
+			fonts.load(el, kElFontDef);
+
+			if(!fonts.empty())
+			{
+				fonts.removeIds();
+				haveFonts = true;
+			}
+		}
+
+		// Get the list definition block
+		else if(name == kElListtable)
+		{
+	        // Load the lists into an id mapped table
+		    lists.load(el, kElListdef);
+
+			if(!lists.empty())
+			{
+				lists.removeIds();
+				haveLists = true;
+			}
+		}
+
+        else if(name == kElBlock)
         {
             // Change style attribute on blocks to name
             if(haveStyles && el.hasAttribute(kElStyle))
@@ -487,6 +486,7 @@ void XmlFixups::runPassTwo(const DOM::Document& doc)
              * new element. The new element still needs to be processed, so we
              * just backup one, and then short circuit the loop below.
              */
+            --it;
 
             // Now fix the block itself
             fixBlock(doc, el);
@@ -602,10 +602,10 @@ void XmlFixups::runPassTwo(const DOM::Document& doc)
         if(parent != NULL && DOMHelpers::hasAncestor(top, node))
         {
             // Remove it from it's child
-            parent.removeChild(el);
+            parent.removeChild(node);
 
             // And put at start of the document of the document
-            top.insertBefore(el, top.getFirstChild());
+            top.insertBefore(node, top.getFirstChild());
         }
 
         toStart.pop();
@@ -619,10 +619,10 @@ void XmlFixups::runPassTwo(const DOM::Document& doc)
         if(parent != NULL && DOMHelpers::hasAncestor(top, node))
         {
             // Remove it from it's child
-            parent.removeChild(el);
+            parent.removeChild(node);
 
             // And put at end of the document of the document
-            top.appendChild(el);
+            top.appendChild(node);
         }
 
         toEnd.pop();
@@ -733,7 +733,7 @@ void XmlFixups::fixBlock(const DOM::Document& doc, DOM::Element& block)
             block.removeAttribute(kAtFix);
 
         // BUG: Sablotron bug work around
-        if(val.length() > 0)
+        if(val.length() == 0)
         {
             val = block.getAttributeNS("", kAtFix);
             if(val.length() > 0)
diff --git a/test-files/sample.xml b/test-files/sample.xml
index 78279ea..a20d97b 100644
--- a/test-files/sample.xml
+++ b/test-files/sample.xml
@@ -1 +1 @@
-<?xml version="1.0" encoding="UTF-8"?><document><stylesheet><style name="Normal"/><style name="heading 1"/><style name="footnote text"/></stylesheet><info><title>This is a test RTF</title><author>Nate</author><operator>Nate</operator></info><para style="heading 1"><i_fonttable><font id="0" name="Times New Roman"/><font id="1" name="Arial"/><font id="2" name="Courier New"/><font id="3" name="Symbol"/><font id="10" name="Wingdings"/><font id="121" name="Times New Roman CE"/><font id="122" name="Times New Roman Cyr"/><font id="124" name="Times New Roman Greek"/><font id="125" name="Times New Roman Tur"/><font id="126" name="Times New Roman (Hebrew)"/><font id="127" name="Times New Roman (Arabic)"/><font id="128" name="Times New Roman Baltic"/><font id="129" name="Times New Roman (Vietnamese)"/><font id="131" name="Arial CE"/><font id="132" name="Arial Cyr"/><font id="134" name="Arial Greek"/><font id="135" name="Arial Tur"/><font id="136" name="Arial (Hebrew)"/><font id="137" name="Arial (Arabic)"/><font id="138" name="Arial Baltic"/><font id="139" name="Arial (Vietnamese)"/><font id="141" name="Courier New CE"/><font id="142" name="Courier New Cyr"/><font id="144" name="Courier New Greek"/><font id="145" name="Courier New Tur"/><font id="146" name="Courier New (Hebrew)"/><font id="147" name="Courier New (Arabic)"/><font id="148" name="Courier New Baltic"/><font id="149" name="Courier New (Vietnamese)"/></i_fonttable><b>This is a test RTF</b></para><para>Hi! I’m a test file. This is some <b>bold</b> text, and some <i>italic</i> text, as well as some <u>underline</u> text. And a bit of <hide>hidden</hide> text. So we’re going to end this paragraph here and go on to a nice little list:</para><para/><list type="disc" ordered="0" start="1"><para>Item 1</para><para>Item 2</para><para>Item 3</para><para>Item 4</para></list><para/><para>And now comes a fun table:</para><para/><table><row><cell><para>Cell 1</para></cell><cell><para>Cell 2</para><para>More in cell 2</para></cell><cell><para>Cell 3</para></cell></row><row><cell><para>Next row</para></cell><cell><para>Next row </para></cell><cell><para>Next row</para></cell></row></table><para/><para>A page break:</para><page/><para>And here we’re on the next page. </para><para>This para has a <ref type="footnote" to="1"><super>1</super></ref>footnote.</para><para>And here’s yet another paragraph. </para><para/><footnote id="1"><para><super>1</super> This is the actual content of the footnote.</para></footnote></document>
+<?xml version="1.0" encoding="UTF-8"?><document><info><title>This is a test RTF</title><author>Nate</author><operator>Nate</operator></info><stylesheet><style name="Normal"/><style name="heading 1"/><style name="footnote text"/></stylesheet><para style="heading 1"><b>This is a test RTF</b></para><para>Hi! I’m a test file. This is some <b>bold</b> text, and some <i>italic</i> text, as well as some <u>underline</u> text. And a bit of <hide>hidden</hide> text. So we’re going to end this paragraph here and go on to a nice little list:</para><para/><list ordered="0" start="1"><para>Item 1</para><para>Item 2</para><para>Item 3</para><para>Item 4</para></list><para/><para>And now comes a fun table:</para><para/><table><row><cell><para>Cell 1</para></cell><cell><para>Cell 2</para><para>More in cell 2</para></cell><cell><para>Cell 3</para></cell></row><row><cell><para>Next row</para></cell><cell><para>Next row </para></cell><cell><para>Next row</para></cell></row></table><para/><para>A page break:</para><page/><para>And here we’re on the next page. </para><para>This para has a <ref type="footnote" to="1"/>footnote.</para><para>And here’s yet another paragraph. </para><para/><footnote id="1"><para> This is the actual content of the footnote.</para></footnote></document>
\ No newline at end of file