[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request: Escape HTML command in window caption before passing to QML
From: Thomas_Lübking <thomas.luebking () web ! de>
Date: 2012-11-23 20:19:07
Message-ID: 20121123201907.3482.34796 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On Nov. 23, 2012, 3:17 p.m., Thomas Lübking wrote:
> > kwin/tabbox/clientmodel.cpp, line 77
> > <http://git.reviewboard.kde.org/r/107431/diff/1/?file=95879#file95879line77>
> >
> > Tested impact on cover/flipswitch?
>
> Martin Gräßlin wrote:
> doesn't have an impact on the effects as they use EffectWindow::caption(). The \
> TabBox Model is not passed to the effects.
http://qt-project.org/doc/qt-5.0/qtquick-porting-qt5.html - see "Behavioral \
Changes"/"Text"
Since the rolenames are more or less dedicated to QML, it *might* be reasonable to \
add an "EscapedCaption" role, bind that to the "caption" name for KDE4 times and \
change the assignement later on (or use an #if QT_VERSION < 0x050000) - this way it \
will not be necesary to cause a behavioral change to the actual model now and another \
one later on (when your tabbox will render "Bash - read < file.txt > \
/dev/null")
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107431/#review22430
-----------------------------------------------------------
On Nov. 23, 2012, 7:38 a.m., Martin Gräßlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107431/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2012, 7:38 a.m.)
>
>
> Review request for kwin.
>
>
> Description
> -------
>
> Escape HTML command in window caption before passing to QML
>
> Escaping is performed in the model instead of the UI as each of the QML
> files needed to be fixed and it is likeley that the issue would come up
> again. In the model it's hopefully fixed for good.
>
> BUG: 309960
> FIXED-IN: 4.9.4
>
>
> This addresses bug 309960.
> http://bugs.kde.org/show_bug.cgi?id=309960
>
>
> Diffs
> -----
>
> kwin/tabbox/clientmodel.cpp a67d979bde622d08285bb27e3824fb2077418233
>
> Diff: http://git.reviewboard.kde.org/r/107431/diff/
>
>
> Testing
> -------
>
> tested with the example named in the bug
>
>
> Thanks,
>
> Martin Gräßlin
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/107431/">http://git.reviewboard.kde.org/r/107431/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On November 23rd, 2012, 3:17 p.m., <b>Thomas \
Lübking</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/107431/diff/1/?file=95879#file95879line77" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/tabbox/clientmodel.cpp</a> <span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">QVariant ClientModel::data(const QModelIndex& index, int role) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">77</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="n">caption</span> <span class="o">=</span> <span \
class="n">Qt</span><span class="o">::</span><span class="n">escape</span><span \
class="p">(</span><span class="n">caption</span><span class="p">);</span></pre></td> \
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested impact on \
cover/flipswitch?</pre> </blockquote>
<p>On November 23rd, 2012, 4:27 p.m., <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">doesn't have an \
impact on the effects as they use EffectWindow::caption(). The TabBox Model is not \
passed to the effects.</pre> </blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">http://qt-project.org/doc/qt-5.0/qtquick-porting-qt5.html - see \
"Behavioral Changes"/"Text"
Since the rolenames are more or less dedicated to QML, it *might* be reasonable to \
add an "EscapedCaption" role, bind that to the "caption" name for \
KDE4 times and change the assignement later on (or use an #if QT_VERSION < \
0x050000) - this way it will not be necesary to cause a behavioral change to the \
actual model now and another one later on (when your tabbox will render "Bash - \
read &lt; file.txt &gt; /dev/null")</pre> <br />
<p>- Thomas</p>
<br />
<p>On November 23rd, 2012, 7:38 a.m., Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for kwin.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Nov. 23, 2012, 7:38 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Escape HTML command in window caption before passing to QML
Escaping is performed in the model instead of the UI as each of the QML
files needed to be fixed and it is likeley that the issue would come up
again. In the model it's hopefully fixed for good.
BUG: 309960
FIXED-IN: 4.9.4</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">tested with the example named in the bug</pre> </td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=309960">309960</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kwin/tabbox/clientmodel.cpp <span style="color: \
grey">(a67d979bde622d08285bb27e3824fb2077418233)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107431/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic