Skip to content
This repository has been archived by the owner on Sep 22, 2020. It is now read-only.

Commit

Permalink
fix: remove page number checking in displaying TOC
Browse files Browse the repository at this point in the history
Former showTOC() method checks the page number in each
entry and only display entry whose page number is greater
than the previous one. However, I think this is too
"clever", we should better leave the TOC untouched to keep
consistent with other pdf readers.
  • Loading branch information
houqp committed Mar 16, 2012
1 parent 9dd3994 commit c8f87d5
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 9 deletions.
5 changes: 3 additions & 2 deletions pdf.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,17 @@ static int walkTableOfContent(lua_State *L, fz_outline* ol, int *count, int dept
lua_pushstring(L, "page");
lua_pushnumber(L, ol->dest.ld.gotor.page + 1);
lua_settable(L, -3);

lua_pushstring(L, "depth");
lua_pushnumber(L, depth);
lua_settable(L, -3);
lua_pushstring(L, "title");

lua_pushstring(L, "title");
lua_pushstring(L, ol->title);
lua_settable(L, -3);

lua_settable(L, -3);

lua_settable(L, -3);
(*count)++;
if (ol->down) {
walkTableOfContent(L, ol->down, count, depth);
Expand Down
10 changes: 3 additions & 7 deletions unireader.lua
Original file line number Diff line number Diff line change
Expand Up @@ -678,15 +678,11 @@ function UniReader:showTOC()
end
local menu_items = {}
local filtered_toc = {}
local curr_page = -1
-- build menu items
for _k,_v in ipairs(self.toc) do
if(_v.page >= curr_page) then
table.insert(menu_items,
(" "):rep(_v.depth-1)..self:cleanUpTOCTitle(_v.title))
table.insert(filtered_toc,_v.page)
curr_page = _v.page
end
table.insert(menu_items,
(" "):rep(_v.depth-1)..self:cleanUpTOCTitle(_v.title))
table.insert(filtered_toc,_v.page)
end
toc_menu = SelectMenu:new{
menu_title = "Table of Contents",
Expand Down

4 comments on commit c8f87d5

@traycold
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I proposed that check because in my case, on a book that I have, the TOC without that check was rendered incorrectly (probably it was corrupted in the PDF itself). I got something like:

Chapter 3
    paragraph1...............pag 23
Chapter 3
    paragraph2...............pag 25
Chapter 3
    paragraph3...............pag 29
Chapter 3
    paragraph4...............pag 31
Chapter 4
    paragraph1...............pag 35
Chapter 4
    paragraph2...............pag 37

and consider that clicking on items Chapter 3 or Chapter 4 I got redirected to page 1 of the document (because probably the items don't have an associated page number), while clicking on paragraph items the behaviour is correct. In this case, including the check of previous item page number, the above TOC is renderer correctly:

Chapter 3
    paragraph1...............pag 23
    paragraph2...............pag 25
    paragraph3...............pag 29
    paragraph4...............pag 31
Chapter 4
    paragraph1...............pag 35
    paragraph2...............pag 37

I don't know if this just something happening on that specific book (and of course in that case it's not relevant at all), or it's something more generic (and in that case such a fix may be useful).

@houqp
Copy link
Member Author

@houqp houqp commented on c8f87d5 Mar 16, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does other pdf readers in your PC also behaviors the same? It is likely to be a corrupted TOC in your PDF file. TOC entries are stored in PDF files as text stream, so you can check this out by opening the PDF file with any text editor. Usually, TOC entries locate at the bottom of the file.

I removed this check because eLiNK reports that in one of his book, kpdfview only displays part of the TOC. I checked his file and it does have a corrupted TOC which has crossed page number. Can you give me your E-Mail address so I can send that file to you for testing? Mine is at https://github.com/houqp :-)

See if we can bring out a patch that fixes both situation.

@traycold
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, I checked out the TOC of my book, and in fact it's corrupted also on my PC viewer (I didn't check this, before), in the sense that you see exactly what you see on TOC of kpdfview. So no need to keep it into account.
Maybe the only improvement we may include is that, in case of "wrong items" in TOC, that don't have a page number attached, we don't go to page 0 by default (i.e. first page), but let that row to be sort of disabled (for instance: skipped during navigation of TOC).

I also added my email address on my profile on github.

Maybe, for testing purposes, could be a nice idea of collecting some "problematic" media documents (PDF and DJVU files having problem with: long loading times; memory issues; problems on TOC or rendering); of course, just not-copyrighted documents.

@houqp
Copy link
Member Author

@houqp houqp commented on c8f87d5 Mar 16, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the only improvement we may include is that, in case of "wrong items" in TOC, that don't have a page number attached, we don't go to page 0 by default (i.e. first page), but let that row to be sort of disabled (for instance: skipped during navigation of TOC).

Agree with that. I have sent you the file in case you want to work on that :)

Maybe, for testing purposes, could be a nice idea of collecting some "problematic" media documents (PDF and DJVU files having problem with: long loading times; memory issues; problems on TOC or rendering); of course, just not-copyrighted documents.

Yes, any suggestion for places to share these files?

EDIT: I added a new issue for it #69

Please sign in to comment.