fix: misplaced subsection position in toc.ncx #13

ChengShusss · 2023-10-16T15:33:56Z

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).

Monirzadeh

Hi @ChengShusss.
thanks for your pull request.
as i see some of test not pass can you check that?
it can be helpful if you add a screenshot from before and after of table of content too.

ChengShusss · 2023-10-23T10:25:45Z

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 · 2023-10-23T10:34:41Z

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 · 2023-10-23T10:36:56Z

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

ChengShusss · 2023-10-23T10:48:23Z

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 · 2023-10-23T11:24:07Z

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 · 2023-10-23T11:56:45Z

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 · 2023-10-23T13:12:00Z

After search, we could get these messages:

toc.ncx is used in EPUB 2, while nav.xhtml is used in EPUB 3;
Current reader prefer to use nav.xhtml than toc.ncx;
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:

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;
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:

Monirzadeh · 2023-10-23T13:12:31Z

Before

After

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 · 2023-10-29T10:09:58Z

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

Monirzadeh · 2023-10-29T10:17:32Z

@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 · 2023-10-29T10:47:04Z

@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 · 2023-10-29T10:51:14Z

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.

fix: misplaced subsection position in toc.ncx (go-shiori#12)

4124542

Monirzadeh requested a review from a team October 23, 2023 09:50

Monirzadeh reviewed Oct 23, 2023

View reviewed changes

fmartingr self-requested a review October 23, 2023 10:52

Monirzadeh mentioned this pull request Oct 23, 2023

if custom filename don't have extension it should return error or automaticaly fix #14

Closed

Monirzadeh changed the title ~~fix: misplaced subsection position in toc.ncx (#12)~~ fix: misplaced subsection position in toc.ncx Oct 23, 2023

Monirzadeh linked an issue Oct 23, 2023 that may be closed by this pull request

Non-proper position for subsection in toc.ncx #12

Closed

Monirzadeh approved these changes Oct 29, 2023

View reviewed changes

Monirzadeh merged commit 8db857c into go-shiori:main Oct 29, 2023
4 checks passed

Monirzadeh added this to the v1.3.0 milestone Dec 25, 2023

Monirzadeh assigned ChengShusss Dec 30, 2023

Monirzadeh mentioned this pull request Mar 2, 2024

feature: allow localizing strings #28

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: misplaced subsection position in toc.ncx #13

fix: misplaced subsection position in toc.ncx #13

ChengShusss commented Oct 16, 2023 •

edited by Monirzadeh

Monirzadeh left a comment •

edited

ChengShusss commented Oct 23, 2023

ChengShusss commented Oct 23, 2023 •

edited by Monirzadeh

Monirzadeh commented Oct 23, 2023

ChengShusss commented Oct 23, 2023

Monirzadeh commented Oct 23, 2023 •

edited

ChengShusss commented Oct 23, 2023

ChengShusss commented Oct 23, 2023

Monirzadeh commented Oct 23, 2023 •

edited

fmartingr commented Oct 29, 2023

Monirzadeh commented Oct 29, 2023

fmartingr commented Oct 29, 2023

fmartingr commented Oct 29, 2023

fix: misplaced subsection position in toc.ncx #13

fix: misplaced subsection position in toc.ncx #13

Conversation

ChengShusss commented Oct 16, 2023 • edited by Monirzadeh

Monirzadeh left a comment • edited

Choose a reason for hiding this comment

ChengShusss commented Oct 23, 2023

ChengShusss commented Oct 23, 2023 • edited by Monirzadeh

Monirzadeh commented Oct 23, 2023

ChengShusss commented Oct 23, 2023

Monirzadeh commented Oct 23, 2023 • edited

ChengShusss commented Oct 23, 2023

ChengShusss commented Oct 23, 2023

Monirzadeh commented Oct 23, 2023 • edited

fmartingr commented Oct 29, 2023

Monirzadeh commented Oct 29, 2023

fmartingr commented Oct 29, 2023

fmartingr commented Oct 29, 2023

ChengShusss commented Oct 16, 2023 •

edited by Monirzadeh

Monirzadeh left a comment •

edited

ChengShusss commented Oct 23, 2023 •

edited by Monirzadeh

Monirzadeh commented Oct 23, 2023 •

edited

Monirzadeh commented Oct 23, 2023 •

edited