go-shiori / go-epub

Go library for creating EPUB files
MIT License
44 stars 7 forks source link

fix: misplaced subsection position in toc.ncx #13

Closed ChengShusss closed 1 year ago

ChengShusss commented 1 year ago

For toc.ncx in epub V2, a subsection navigate label should be placed under section label, while subsection is misplaced parallel to section.

This is because function toc.addSubSection (toc.go: 173) used opposite condition for ncxXML. When there is parent section for subsection, code goes into a situation assuming no parent section, which means taking this subsection as a lonely section.

So, simply exchange the condition would work. And for code style consistency, use same style like navXML part (toc.go: 191).

ChengShusss commented 1 year ago

图片 It seems like that some dependency can not be download as expect. Since I do not use mac, is there any alternative linux (e.g. ubuntu) platform for err check?

ChengShusss commented 1 year ago

图片 It seems like that some dependency can not be download as expect. Since I do not use mac, is there any alternative linux (e.g. ubuntu) platform for err check?

As we can see in file change tab, this pull request only change one line.

Monirzadeh commented 1 year ago

can you send an screenshot of table of content before after too?

ChengShusss commented 1 year ago

Sorry to misunderstand the meaning of table of content, here is the difference before and after:

demo code:

package main

import (
    "fmt"
    "log"

    epub "github.com/go-shiori/go-epub"
)

func main() {
    // Create a new EPUB
    e, err := epub.NewEpub("My title")
    if err != nil {
        log.Println(err)
    }

    // Add a section
    sectionBody := `<h1>Section 1</h1>
    <p>This is a paragraph.</p>`
    parent1, err := e.AddSection(sectionBody, "Section 1", "", "")
    if err != nil {
        log.Println(err)
    }

    // Add a subsection
    subsectionBody := `<h2>Section 1-1</h2>
    <p>This is a paragraph of h2.</p>`
    _, err = e.AddSubSection(parent1, subsectionBody, "Section 1-1", "SubSectionName", "")
    if err != nil {
        log.Println(err)
    }

    // Write the EPUB
    err = e.Write("My EPUB.epub")
    if err != nil {
        // handle error
        fmt.Printf("Falied to create epub, err: %v\n", err)
    }
}

toc.ncx before modify:

<?xml version="1.0" encoding="UTF-8"?>
<ncx xmlns="http://www.daisy.org/z3986/2005/ncx/" version="2005-1">
  <head>
    <meta name="dtb:depth" content="urn:uuid:be83a5f5-2258-4c7f-9cda-f536aab9bf71"></meta>
  </head>
  <docTitle>
    <text>My title</text>
  </docTitle>
  <docAuthor>
    <text></text>
  </docAuthor>
  <navMap>
    <navPoint id="navPoint-0">
      <navLabel>
        <text>Section 1</text>
      </navLabel>
      <content src="xhtml/section0001.xhtml"></content>
    </navPoint>
    <navPoint id="navPoint-1">
      <navLabel>
        <text>Section 1-1</text>
      </navLabel>
      <content src="xhtml/SubSectionName"></content>
    </navPoint>
  </navMap>
</ncx>

And this is after:

<?xml version="1.0" encoding="UTF-8"?>
<ncx xmlns="http://www.daisy.org/z3986/2005/ncx/" version="2005-1">
  <head>
    <meta name="dtb:depth" content="urn:uuid:7af96619-4e81-4ae4-a39d-f98384e5a4ca"></meta>
  </head>
  <docTitle>
    <text>My title</text>
  </docTitle>
  <docAuthor>
    <text></text>
  </docAuthor>
  <navMap>
    <navPoint id="navPoint-0">
      <navLabel>
        <text>Section 1</text>
      </navLabel>
      <content src="xhtml/section0001.xhtml"></content>
      <navPoint id="navPoint-1">
        <navLabel>
          <text>Section 1-1</text>
        </navLabel>
        <content src="xhtml/SubSectionName"></content>
      </navPoint>
    </navPoint>
  </navMap>
</ncx>
Monirzadeh commented 1 year ago

Now i can see better your change. but visually and functionality on my eBook reader are the same as each other as you mention in issue

for proper show in reader

do you test that in specific device and any specific program not show them same as each other? i test that with calibre, zathura and koreader. your first point is right. subsection now place in correct position.

ChengShusss commented 1 year ago

It is tested with sumatraPDF to check table of content, and on sumatraPDF, it acts expectantly. Maybe it is because epub has several ways to gain toc and sumatra PDF just take toc.ncx, while other readers do not use this method?

Please give me some time to find out why.

ChengShusss commented 1 year ago

After search, we could get these messages:

  1. toc.ncx is used in EPUB 2, while nav.xhtml is used in EPUB 3;
  2. Current reader prefer to use nav.xhtml than toc.ncx;
  3. SumatraPDF-3.4.6 use NCX for navigate(this is strange), which brought up the problem that bothered me in the first place - unexpected subsection.

Some experiment:

  1. Open generated epub file (use go-epub before commit) with calibre/sumatraPDF 3.3.3, table of content is in order already;but with sumatraPDF 3.4.6 (latest release version), subsection is in wrong place;
  2. Modify nav.xhtml (this file is for navigation for EPUB 3) in epub file, and open it with reader, calibre/sumatraPDF 3.3.3 shows the modification, while sumatraPDF 3.4.6 dose not.

Then we can summarize: Only reader parse epub as EPUB2 (use toc.ncx), the difference would show up.

That's all and thanks for you valuable question!

ref:

  1. EPUB 3.3 - ncx
  2. Open Packaging Format (OPF) 2.0.1 v1.0
Monirzadeh commented 1 year ago

Before 81a1dccd-8e37-4a51-aee2-cc4c15f21695 png After c9847cb8-a8d1-48eb-948c-f2506322a588 png

In sumatraPDF or readers limit to EPUB 2 we will merge this as soon as we find problem with test-mac-windows (macos-latest) thanks @ChengShusss

fmartingr commented 1 year ago

@Monirzadeh @ChengShusss I see tests passing at the moment, is there anything specific for me to check here?

Monirzadeh commented 1 year ago

@fmartingr at the moment we get this https://user-images.githubusercontent.com/34236969/277307095-799a7430-cb0a-426f-9ca7-4c83f22343a3.png maybe it is something random happen and it is not related to the test itself

fmartingr commented 1 year ago

@fmartingr at the moment we get this https://user-images.githubusercontent.com/34236969/277307095-799a7430-cb0a-426f-9ca7-4c83f22343a3.png maybe it is something random happen and it is not related to the test itself

Let me run the test locally to check, but the CI is passing at the moment.

fmartingr commented 1 year ago

Tests ran correctly on my mac, and I re-run all tests here in Github and passed as well. I think it was a network error at the time.