[Kde-pim] Re: Review Request: Fix for Bug 207779 - Kmail removes empty lines in HTML

[Thomas McGuire]

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


Please add a unit test for this.
Any reason to add this to KMEditor instead of its baseclass KRichTextEdit?
There is an empty line after each code line, that makes the code harder to read, IMHO you should remove some of the empty lines.

BTW, the best way to fix this would be fixing this in Qt itself or to revive our own HTML generator in kdepimlibs/kpimtextedit/richtextbuilders, but both ways are probably too much work.

Thanks for the patch!


messagecomposer/kmeditor.cpp
<http://git.reviewboard.kde.org/r/100281/#comment497>

    make this const: const QString textEditHtml = ...
    
    Many other variables can be const as well in this function.



messagecomposer/kmeditor.cpp
<http://git.reviewboard.kde.org/r/100281/#comment494>

    QString::fromAscii( "" ) -> QString()



messagecomposer/kmeditor.cpp
<http://git.reviewboard.kde.org/r/100281/#comment495>

    Simply "return webframe->toHtml()" would be easier here.


- Thomas


On 2011-01-01 18:48:53, Frank Vanderham wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100281/
> -----------------------------------------------------------
> 
> (Updated 2011-01-01 18:48:53)
> 
> 
> Review request for KDEPIM.
> 
> 
> Summary
> -------
> 
> The KMail message editor uses Qt's rich text editor functionality. The HTML markup generated by this control is not compatible with email readers such as MS Outlook. This fix intercepts the HTML markup generated by the editor and fixes the markup before sending it out.
> The HTML parser used to fix the markup is Qt's Webkit, which, at first blush, sounds "heavy", but as discussed on IRC a couple of days ago, there is no performance loss and WebKit is already linked to Kmail, so there should be no side-effects.
> 
> [Personal note: I need to stress that as end-user of KDE for business purposes myself, this is a huge problem with KMail today: my customers complain my KMail generated email looks terrible when received, which is what prompted me to roll up my sleeves and fix it myself.]
> 
> 
> This addresses bug 207779.
>     http://bugs.kde.org/show_bug.cgi?id=207779
> 
> 
> Diffs
> -----
> 
>   messagecomposer/kmeditor.h af9bf89 
>   messagecomposer/kmeditor.cpp 7ab1288 
>   messagecomposer/CMakeLists.txt 477bc29 
> 
> Diff: http://git.reviewboard.kde.org/r/100281/diff
> 
> 
> Testing
> -------
> 
> I stood up a MS system with Outlook Express and send through a number of HTML formatted emails, especially with "empty lines", bullet lists and numbered lists (which all show up mangled prior to the fix). All those emails arrived as desired.
> 
> After Torgny's comment, I also added a test case for Thunderbird: email arrived as desired. Of course, the mail viewed in KMail also looks as required.
> 
> 
> Thanks,
> 
> Frank
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/