[Kde-pim] Review Request: Processed and HTML sources tabs added to MessageSourceViewer

[Thomas McGuire]

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

Ship it!


Very nice idea, definitely useful for debugging!
I don't know better titles for the tabs either.
But maybe tooltips on the tab title could be useful, but then, I wouldn't know what text to add to those as well :)
Something along the lines of:
1) Raw, unmodified mail as it is stored on the filesystem or on the server 
(well not entirely true since it might be that the mail is missing attachments for attachment loading on demand)
2) Processed mail, for example after decrypting an encrypted part of the mail
3) HTML code for displaying the message to the user


/trunk/KDE/kdepim/messageviewer/mailsourceviewer.h
<http://reviewboard.kde.org/r/2730/#comment3263>

    QString should be passed by reference, that is slightly faster (even though QString is implicitly shared, which makes passing by value quite fast already, but nothing can beat passing by reference)
    
    so: ( const QString &source )
    Same for others



/trunk/KDE/kdepim/messageviewer/mailsourceviewer.h
<http://reviewboard.kde.org/r/2730/#comment3265>

    Can you name these mRawBrowser or something like that instead?
    I was reading the diff below and got confused because I thought this would be a KMime::Message.



/trunk/KDE/kdepim/messageviewer/mailsourceviewer.cpp
<http://reviewboard.kde.org/r/2730/#comment3264>

    deleting shouldn't be necessary: QSyntaxHighlighter is a QObject, which means the sytax highlighter will get deleted as soon as the parent object gets deleted.



/trunk/KDE/kdepim/messageviewer/viewer_p.cpp
<http://reviewboard.kde.org/r/2730/#comment3266>

    Heh, thanks for removing the comment, I was just about to remark that :)


- Thomas


On 2010-01-25 20:00:36, Torgny Nyblom wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2730/
> -----------------------------------------------------------
> 
> (Updated 2010-01-25 20:00:36)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Add two tabs to the MessageSourceViewer. One for the source after being processed and one for the html source used to render the message.
> I think these two can be useful but perhaps not for everyone. An option would be to hide these tabs behind a hidden setting (developerExtras or something like that), but where should this setting go? I didn't find where to add this inside the MessageViewer.
> 
> There might be better texts for the tab labels.
> 
> 
> This addresses bug 223845.
>     https://bugs.kde.org/show_bug.cgi?id=223845
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/messageviewer/mailsourceviewer.h 1080057 
>   /trunk/KDE/kdepim/messageviewer/mailsourceviewer.cpp 1080057 
>   /trunk/KDE/kdepim/messageviewer/viewer_p.cpp 1080139 
> 
> Diff: http://reviewboard.kde.org/r/2730/diff
> 
> 
> Testing
> -------
> 
> Sources shown in KMail.
> 
> 
> Thanks,
> 
> Torgny
> 
>

_______________________________________________
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/