Review Request 108925: Fix Mobi and Epub filters problems

[Inge Wallin]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108925/#review27367
-----------------------------------------------------------



filters/words/epub/OdtHtmlConverter.cpp
<http://git.reviewboard.kde.org/r/108925/#comment20586>

    I think correct English is "we man not", not "we may don't".  But that aside, bookmarkFlag is not a good name since it doesn't say what the bool represents.  Under which conditions is it set and under which is it not?



filters/words/epub/exportepub2.cpp
<http://git.reviewboard.kde.org/r/108925/#comment20587>

    "This" is completely unclear for the user when he runs the filter and gets this error.  You should at least tell the user what the name is and that it's ignored. 



filters/words/mobi/OdtMobiHtmlConverter.cpp
<http://git.reviewboard.kde.org/r/108925/#comment20588>

    Does this mean that you now ignore end of bookmarks?  If so, that should probably be documented.



filters/words/mobi/PalmDocCompression.cpp
<http://git.reviewboard.kde.org/r/108925/#comment20590>

    we normally use 0x09, not 0X09.  And why the magic number 0x09? Document!  :)



filters/words/mobi/exportmobi.cpp
<http://git.reviewboard.kde.org/r/108925/#comment20591>

    Unclear kdebug that says nothing to the user when he sees it. Even a developer gets no information from this.


Ok, here are some comments.  I think that you should separate the commit that fixes the crash, including comments from the ones that fixes the external images issue.  To mix them up like this is generally not good. But these are all small issues and it's almost ready for merge.

- Inge Wallin


On Feb. 12, 2013, 5:05 p.m., mojtaba shahi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108925/
> -----------------------------------------------------------
> 
> (Updated Feb. 12, 2013, 5:05 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> I tested all documents that frinring sent to me, and found some problems in mobi (algorithm and in converting odf to mobi html) and in extracting
> images (external images) and in epub (bad handleing in tag bookmark).
> 
> Fixed all problems.
> 
> 
> Diffs
> -----
> 
>   filters/words/epub/OdtHtmlConverter.cpp 6564cd3 
>   filters/words/epub/exportepub2.cpp 461cc9c 
>   filters/words/mobi/OdtMobiHtmlConverter.cpp de5ff3f 
>   filters/words/mobi/PalmDocCompression.cpp 1eff462 
>   filters/words/mobi/exportmobi.cpp fee2509 
> 
> Diff: http://git.reviewboard.kde.org/r/108925/diff/
> 
> 
> Testing
> -------
> 
> Tested with all documents that sent to me, all documents exported ok and without any problem.
> 
> 
> Thanks,
> 
> mojtaba shahi
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130213/c9e9496e/attachment.htm>