Multi-level TOC broken in epub->epub conversion

[siebert]

Hi,

I'm taking the sample xhtml code from the calibre manual at
http://calibre-ebook.com/user_manual...le-of-contents
and save it as a html file, add the file to calibre as an ebook (which results in a zip file), enable "Force use of auto-generated Table of Contents", set level1 and level2 toc values as described and convert to epub.

The resulting epub contains a two-level TOC as expected, but if I convert this epub again to epub using the same setting, the TOC of the new epub contains only the level1 headings. Bummer.

Ciao,
Steffen

[Manichean]

I'm guessing you'll have to adjust the level 2 detection XPath to account for changes occuring during conversion. Have a look at the XHTML inside the ePub to see what the relevant headings are tagged like.

[siebert]

Originally Posted by Manichean

I'm guessing you'll have to adjust the level 2 detection XPath to account for changes occuring during conversion. Have a look at the XHTML inside the ePub to see what the relevant headings are tagged like.

Nope, that's not the reason. In fact, if I move the level 2 XPath to level 1 and delete the input field for level 2, I get a single-level TOC containing the level 2 headings, so this is not the cause.

Ciao,
Steffen

[kovidgoyal]

the level 2 expression only matches if a leve1 expression matched in the same html file.

Why are you forcing use of auto generated toc twice anyway?

[siebert]

Originally Posted by kovidgoyal

the level 2 expression only matches if a leve1 expression matched in the same html file.

I was afraid that the splitting might be the reason.

Is there a way to have calibre merge the html files again?

Why are you forcing use of auto generated toc twice anyway?

Because it took several iterations until I had figured out how to create the TOC the way I wanted and I also overlooked some headings in my first attempt.

Ciao,
Steffen

[kovidgoyal]

No calibre doesn't do merging.

[Manichean]

Originally Posted by siebert

Because it took several iterations until I had figured out how to create the TOC the way I wanted and I also overlooked some headings in my first attempt.

Why not just correct the XPath and reconvert from HTML, then?

[siebert]

Originally Posted by Manichean

Why not just correct the XPath and reconvert from HTML, then?

Because in the real case I've started with an epub, not a html file.

Ciao,
Steffen

[siebert]

Originally Posted by kovidgoyal

No calibre doesn't do merging.

That's a pity. But it should be possible to implement, or does calibre discard some information it won't be able to recover?

Originally Posted by kovidgoyal

the level 2 expression only matches if a leve1 expression matched in the same html file.

As a proper formatted epub won't match this requirement, it should be considered a bug and fixed, shouldn't it?

Ciao,
Steffen

[kovidgoyal]

Originally Posted by siebert

That's a pity. But it should be possible to implement, or does calibre discard some information it won't be able to recover?

It's certainly possible, just not something I care to implement.

As a proper formatted epub won't match this requirement, it should be considered a bug and fixed, shouldn't it?

Ciao,
Steffen

A properly formatted epub will also have a properly formatted TOC. Again, it's perfectly possible to have the code remember the last encountered level n-1 TOC entry and use that for the next level n toc entry across HTML flows, but again, it's not something I consider important enough to spend time on.

[siebert]

Originally Posted by kovidgoyal

A properly formatted epub will also have a properly formatted TOC.

It might. But if I want to use calibre to generate a different TOC according to the rules I define in calibre, I consider it a bug if it doesn't work as documented.

Again, it's perfectly possible to have the code remember the last encountered level n-1 TOC entry and use that for the next level n toc entry across HTML flows, but again, it's not something I consider important enough to spend time on.

I find bugs that make a conversion degrading not so unimportant.

Ciao,
Steffen

[kovidgoyal]

Then feel free to submit a patch

[siebert]

Originally Posted by kovidgoyal

Then feel free to submit a patch

Ok, here it is:

diff --git a/src/calibre/ebooks/oeb/transforms/structure.py b/src/calibre/ebooks/oeb/transforms/structure.py
index 0db9b15..90de41d 100644
--- a/src/calibre/ebooks/oeb/transforms/structure.py
+++ b/src/calibre/ebooks/oeb/transforms/structure.py
@@ -95,10 +95,8 @@ class DetectStructure(object):
                     self.log.exception('Failed to mark chapter')
 
     def create_level_based_toc(self):
-        if self.opts.level1_toc is None:
-            return
-        for item in self.oeb.spine:
-            self.add_leveled_toc_items(item)
+        if self.opts.level1_toc is not None:
+            self.add_leveled_toc_items()
 
     def create_toc_from_chapters(self):
         counter = self.oeb.toc.next_play_order()
@@ -145,14 +143,15 @@ class DetectStructure(object):
         return text, href
 
 
-    def add_leveled_toc_items(self, item):
-        level1 = XPath(self.opts.level1_toc)(item.data)
+    def add_leveled_toc_items(self):
         level1_order = []
-        document = item
-
+        added = {}
+        added2 = {}
         counter = 1
-        if level1:
-            added = {}
+        for item in self.oeb.spine:
+            level1 = XPath(self.opts.level1_toc)(item.data)
+            document = item
+
             for elem in level1:
                 text, _href = self.elem_to_link(document, elem, counter)
                 counter += 1
@@ -163,14 +162,18 @@ class DetectStructure(object):
                     added[elem] = node
                     #node.add(_('Top'), _href)
             if self.opts.level2_toc is not None:
-                added2 = {}
                 level2 = list(XPath(self.opts.level2_toc)(document.data))
                 for elem in level2:
                     level1 = None
                     for item in document.data.iterdescendants():
                         if item in added.keys():
                             level1 = added[item]
-                        elif item == elem and level1 is not None:
+                        elif item == elem:
+                            if level1 is None:
+                                if added == {}:
+                                    continue
+                                else:
+                                    level1 = added[added.keys()[-1]]
                             text, _href = self.elem_to_link(document, elem, counter)
                             counter += 1
                             if text:
@@ -183,11 +186,15 @@ class DetectStructure(object):
                         for item in document.data.iterdescendants():
                             if item in added2.keys():
                                 level2 = added2[item]
-                            elif item == elem and level2 is not None:
+                            elif item == elem:
+                                if level2 is None:
+                                    if added2 == {}:
+                                        continue
+                                    else:
+                                        level2 = added2[added2.keys()[-1]]
                                 text, _href = \
                                         self.elem_to_link(document, elem, counter)
                                 counter += 1
                                 if text:
                                     level2.add(text, _href,
-                                    play_order=self.oeb.toc.next_play_order())
-
+                                        play_order=self.oeb.toc.next_play_order())

Ciao,
Steffen

[kovidgoyal]

Your patch will not work for the following reasons:

1) You need to use an OrderedDict not {}

2) if a file contains an <h2> as the first element and an <h1> after it, then the <h2> will be added to the <h1> from that file instead of the the <h1> from the previous file

I have fixed both issues, but I haven't really tested my fix, so let me know if there are any problems.

[siebert]

Originally Posted by kovidgoyal

Your patch will not work for the following reasons:

It worked at least for my test documents and the real book I wanted to convert in the first place.

1) You need to use an OrderedDict not {}

That's correct. On my python implementation the simple dictionary seems to work the same way as the OrderedDict, but only by accident.

2) if a file contains an <h2> as the first element and an <h1> after it, then the <h2> will be added to the <h1> from that file instead of the the <h1> from the previous file

Also correct, I didn't thought of that case during my tests.

I've now created such an epub with sigil and your implementation creates the correct TOC, while mine misplaces the <h2> as described.

I have fixed both issues, but I haven't really tested my fix, so let me know if there are any problems.

Thanks, it works fine for me.

Ciao,
Steffen