Review Request 111746: Don't let HTML-like filenames be interpreted as HTML strings

Mon Jul 29 09:33:01 BST 2013

> On July 28, 2013, 1:03 p.m., Dawit Alemayehu wrote:
> > dolphin/src/dolphinpart.cpp, line 61
> > <http://git.reviewboard.kde.org/r/111746/diff/1/?file=173960#file173960line61>
> >
> >     Unnecessary include?
> 
> Frank Reininghaus wrote:
>     At first sight, the include looks unnecessary indeed. Fabio, if it still builds without the include, just remove it and feel free to push this commit. IMHO, it looks safe enough for the KDE/4.11 branch. Thanks for your work!

The include is necessary, this is where Qt::escape is defined. This strangeness has been fixed in Qt 5 (QString::toHtmlEscaped)

- David

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

On July 28, 2013, 8:06 a.m., Fabio D'Urso wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111746/
> -----------------------------------------------------------
> 
> (Updated July 28, 2013, 8:06 a.m.)
> 
> 
> Review request for Dolphin, David Faure and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> The attached diff fixes the same issue in two places: filenames looking like HTML (eg "<html>H<font color=red>ello") are displayed as if they were HTML strings on hover in the status bar, both in dolphin and in konqueror (see attached screenshot).
> 
> Note that the status bar text is handled in different ways:
> 
> The dolphin/src/statusbar/dolphinstatusbar.cpp change fixes the dolphin case. In dolphin, the status bar text is directly set by DolphinViewContainer::showItemInfo. This patch just configures dolphin's statusbar to use Qt::PlainText.
> 
> In konqueror, instead, the status bar text comes from DolphinPart::slotRequestItemInfo, which emits the KPart's setStatusBarText signal. The attached patch fixes the issue by escaping the string before emitting it.
> I have a doubt about this, and this is the reason why I'm involving dfaure: is setStatusBarText actually supposed to carry HTML data? Konqueror interprets setStatusBarText's text as HTML, if it starts with "<qt>" or "<html>" (KonqStatusBarMessageLabel::Private::isRichText).
> 
> 
> This addresses bug 321778.
>     http://bugs.kde.org/show_bug.cgi?id=321778
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphinpart.cpp e8138eb 
>   dolphin/src/statusbar/dolphinstatusbar.cpp 6f734ed 
> 
> Diff: http://git.reviewboard.kde.org/r/111746/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> Screenshot of the issue
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/07/28/bug321778-screenshot.png
> 
> 
> Thanks,
> 
> Fabio D'Urso
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130729/789d4524/attachment.htm>