[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [calligra/export-epub-moji] filters/words/epub: Even more cleaning.
From:       Inge Wallin <inge () lysator ! liu ! se>
Date:       2012-07-31 23:03:50
Message-ID: 20120731230350.36390A6094 () git ! kde ! org
[Download RAW message or body]

Git commit 046447c35fc2a656e50b08001e9d2c0c39218e89 by Inge Wallin.
Committed on 01/08/2012 at 01:03.
Pushed by ingwa into branch 'export-epub-moji'.

Even more cleaning.

M  +46   -35   filters/words/epub/htmlconvert.cpp

http://commits.kde.org/calligra/046447c35fc2a656e50b08001e9d2c0c39218e89

diff --git a/filters/words/epub/htmlconvert.cpp b/filters/words/epub/htmlconvert.cpp
index 4425330..225ed2b 100644
--- a/filters/words/epub/htmlconvert.cpp
+++ b/filters/words/epub/htmlconvert.cpp
@@ -420,34 +420,22 @@ QPair<KoFilter::ConversionStatus, bool> convertContent(KoStore *odfStore, QHash<
         //kDebug(30517) << nodeElement.tagName() <<pFlag;
 
         //  Check for tag table
-        if (nodeElement.tagName() == "table") {
-            handleTagTable(nodeElement, bodyWriter);
+        if (nodeElement.tagName() == "p" || nodeElement.tagName() =="h") {
 
-            // FIXME: I don't understand this at all. /IW
-            if (pFlag) {
-                if (childNode.nextSibling().isNull()) {
-                   childNode = pNode;
-                   nodeElement = childNode.toElement();
-                   pFlag = false;
-                }
-            }
-        } 
-        else if (nodeElement.tagName() == "p" || nodeElement.tagName() =="h") {
-            // A break-before in the style means create a new chapter here.
+            // A break-before in the style means create a new chapter here,
+            // but only if it is a top-level paragraph.
             if (breakBeforeStylesList.contains(nodeElement.attribute("style-name"))
-                && startNode != sibl)
+                && startNode != sibl
+                && !pFlag)
             {
-                // We should check that this is root paragraph
-                if (!pFlag) {
-                    // This paragraph is at top level so we should close the html file and return.
-                    bodyWriter->endElement();
-                    bodyWriter->endElement();
-                    // Save this node for next html file.
-                    startNode = sibl;
-                    // Return
-                    odfStore->close();
-                    return qMakePair(KoFilter::OK, true);
-                }
+                // This paragraph is at top level so we should close the html file and return.
+                bodyWriter->endElement();
+                bodyWriter->endElement();
+                // Save this node for next html file.
+                startNode = sibl;
+                // Return
+                odfStore->close();
+                return qMakePair(KoFilter::OK, true);
             }
 
             if (nodeElement.tagName() == "p")
@@ -524,36 +512,49 @@ QPair<KoFilter::ConversionStatus, bool> convertContent(KoStore *odfStore, QHash<
            childNode = childNode.nextSibling();
            nodeElement = childNode.toElement();
         }
+        else if (nodeElement.tagName() == "table") {
+            // Handle table
 
-        // Handle frames
+            handleTagTable(nodeElement, bodyWriter);
+
+            // FIXME: I don't understand this at all. /IW
+            if (pFlag) {
+                if (childNode.nextSibling().isNull()) {
+                    childNode = pNode;
+                    nodeElement = childNode.toElement();
+                    pFlag = false;
+                }
+            }
+        } 
         else if (nodeElement.tagName() == "frame")  {
+            // Handle frame
+
             if (!pFlag) {
                 bodyWriter->startElement("div");
-                bodyWriter->startElement("img");
             }
-            else
-                bodyWriter->startElement("img");
+
+            bodyWriter->startElement("img");
             bodyWriter->addAttribute("class", nodeElement.attribute("style-name"));
             bodyWriter->addAttribute("alt", "(No Description)");
 
             // Check image tag to find image source
-            QDomNode imgNode = childNode.firstChild();
-            QDomElement imgElement = imgNode.toElement();
+            QDomElement imgElement = childNode.firstChild().toElement();
 
             QString imgSrc = imgElement.attribute("href").section('/', 1); // I like section :)
             bodyWriter->addAttribute("src", imgSrc);
             //bodyWriter->endElement();
 
             if (pFlag) {
+                // FIXME: What's going on here?? /IW
                 if (childNode.nextSibling().isNull()) {
-                    bodyWriter->endElement();
+                    bodyWriter->endElement(); // Why isn't the img tag *always* ended?
                     childNode = pNode;
                     nodeElement = childNode.toElement();
                     pFlag = false;
                 }
             }
             else
-                bodyWriter->endElement();
+                bodyWriter->endElement(); // div
         }
         else if (nodeElement.tagName() == "soft-page-break") {
             bodyWriter->addTextNode(nodeElement.text().toUtf8());
@@ -580,6 +581,15 @@ QPair<KoFilter::ConversionStatus, bool> convertContent(KoStore *odfStore, QHash<
             nodeElement = childNode.toElement();
             pFlag = false;
         }
+
+        // FIXME: This is a bad optimization that makes the code super
+        //        difficult to follow!  Wherever there is a
+        //        startElement() there should also be an
+        //        endElement(). The calls to endElement() should not
+        //        be optimized this way. It leads to bugs. For
+        //        instance, the case "Ignore unknown tag" above has no
+        //        call to startElement() at all. Yet we have this here
+        //        endElement(). :(
         bodyWriter->endElement(); // tagName()
 
         if (!pFlag)
@@ -590,7 +600,6 @@ QPair<KoFilter::ConversionStatus, bool> convertContent(KoStore *odfStore, QHash<
     }
 
     bodyWriter->endElement(); // body
-
     bodyWriter->endElement(); // html
 
     odfStore->close();
@@ -718,5 +727,7 @@ void handleTagTable(QDomElement &nodeElement, KoXmlWriter *bodyWriter)
         tableElement = tableNode.toElement();
     } // end while write tag row
 
-    bodyWriter->endElement(); // table
+    // FIXME: Apparently the tag is ended in the main loop. 
+    //        That "optimization" is bad and should be removed.
+    //bodyWriter->endElement(); // table
 }
[prev in list] [next in list] [prev in thread] [next in thread]