julien-c / epub

node.js epub reader
https://www.npmjs.com/package/epub
Other
335 stars 307 forks source link

fix toc parser #13

Closed mawi12345 closed 9 years ago

mawi12345 commented 9 years ago

This PR overcomes the empty text edge case in:

title = (branch[i].navLabel && branch[i].navLabel.text || branch[i].navLabel || "").trim();

if branch[i].navLabel is {text: ''} then this will result in TypeError: Object #<Object> has no method 'trim'

Because the branch[i].navLabel.text is an empty string and evaluates to false in the if clause.


Furthermore i have moved the variable definition in the for loop.

mawi12345 commented 9 years ago

btw i have tested the code on a ebook collection of 60.000 books. The only case where ebook-meta can handle files the do not work with your lib are related to wrong zip headers.

0000000   P   K 003 004 024  \0 004  \0  \0  \0 003 005   C   <   o   a
0000010   ?   , 024  \0  \0  \0 024  \0  \0  \0  \b  \0  \0  \0   m   i
0000020   m   e   t   y   p   e   a   p   p   l   i   c   a   t   i   o
0000030   n   /   e   p   u   b   +   z   i   p   P   K 003 004 024  \0

the file should start with P K 001 002 (central directory header) and not with P K 004 005 (local file header)

If i find time i will try to fork adm-zip and fix the problem there.

julien-c commented 9 years ago

Do you mind just adding curly brackets around ifs to have a consistent style?

Looks good to me otherwise.

mawi12345 commented 9 years ago

oh sorry, now it should be consistent