<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/108677/">http://git.reviewboard.kde.org/r/108677/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 1st, 2013, 11:36 p.m. UTC, <b>Jarosław Staniek</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/108677/diff/2/?file=111331#file111331line84" style="color: black; font-weight: bold; text-decoration: underline;">kexi/kexiutils/KexiPushButton.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<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">84</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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;">Shouldn't we add here:
if (executable) { KRun::run(....); }</pre>
</blockquote>
<p>On February 2nd, 2013, 3:20 p.m. UTC, <b>Oleg Kukharchuk</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;">You think KRun::runUrl(url, type, q); not enough for that? Of course we should use defaultHyperlinkTool for that.</pre>
</blockquote>
<p>On February 2nd, 2013, 11:22 p.m. UTC, <b>Jarosław Staniek</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;">You're right, no need to cover special case. I thought I found an obscure case but cannot reproduce that. If found, can be fixed later. Both static and dynamic case works as well as urls like file:///usr/bin/kate.
I have related points thought:
- a note: we shall think about security, not counting scripts, this feature is the first one that has access to executing arbitrary code
- how about defaulting to only relative paths? for that we'd need another property
- however, we're not supporting arguments, so we're not very unsafe so far; if we were, /usr/bin/rm -rf ~ would be possible
- btw, why not supporting args, this would be usable feature, easy to implement, and users would be able to skip a need for programming in such case
- for dynamic hyperlink type, how about setting the button's text (in Data mode only) to the hyperlink text?
</pre>
</blockquote>
<p>On February 5th, 2013, 12:12 p.m. UTC, <b>Oleg Kukharchuk</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;">Hm.. I think if the user created the database by himself he/she knows what he/she do. And he could execute /usr/bin/rm -rf ~ in terminal and it's more simple. Isn't it? But if the database is a "gift" from "good" man it's really can be dangerous. But what we can do here? (maybe a warning for the user?)
about defaulting relative paths. Is there any reason for that?
Due to security reasons I would prefer to keep it without args. We should be carefully here.
And about button's text. when I started coding I added code for setting hyperlink as description for command link button. But I faced problems with button's repainting, so I've removed this code(I'll think about that). and one more. hyperlink can be very long and it's not very good for button text. what do you think about that?</pre>
</blockquote>
<p>On February 6th, 2013, 8:14 p.m. UTC, <b>Jarosław Staniek</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;">> Hm.. I think if the user created the database by himself he/she knows what he/she do.
> And he could execute /usr/bin/rm -rf ~ in terminal and it's more simple. Isn't it?
> But if the database is a "gift" from "good" man it's really can be dangerous.
> But what we can do here? (maybe a warning for the user?)
> about defaulting relative paths. Is there any reason for that?
How about warning the user and let her to accept the risk by checking an "Allow" checkbox? That could be done with idea of safe projects; a safe project has higher privileges.
> Due to security reasons I would prefer to keep it without args. We should be carefully here.
We can leave that as a TODO.
> And about button's text. when I started coding I added code for setting hyperlink
> as description for command link button. But I faced problems with button's repainting,
> so I've removed this code(I'll think about that).
> and one more. hyperlink can be very long and it's not very good for button text.
If it's too long we can:
- display elided text using QFontMetrics::elidedText(text, Qt::ElideMiddle, ...) and QPainter::drawText()
- or display the filename if the path leads to filename (can be combined with elided text too)
Both things at lease for kexidbpushbutton can be implemented using the standard QPushButton::setText() method. I need to see the problematic code you're talking about.
We always can do extra thing: setting a tooltip to the full text.
PS: A future extension (no idea if we discussed that): http://community.kde.org/Kexi/Plugins/Forms/Button_hyperlinks#Appearance_property
</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;">Added KexiHyperLinkHandler class
> How about warning the user and let her to accept the risk by checking an "Allow" checkbox?
message box added.
> We can leave that as a TODO.
keep as TODO
when hyperlink type is Dynamic - set hyperlink as text of the button (elided) or as description for the commandlink button.
> PS: A future extension (no idea if we discussed that): http://community.kde.org/Kexi/Plugins/Forms/Button_hyperlinks#Appearance_property
keep as TODO
</pre>
<br />
<p>- Oleg</p>
<br />
<p>On May 29th, 2013, 8 p.m. UTC, Oleg Kukharchuk wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Calligra, Adam Pigg, Jarosław Staniek, and Dimitrios Tanis.</div>
<div>By Oleg Kukharchuk.</div>
<p style="color: grey;"><i>Updated May 29, 2013, 8 p.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;">added support for hyperlinks in kexidbpushbutton and kexicommandlinkbutton</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kexi/kexiutils/CMakeLists.txt <span style="color: grey">(943eae1f147c65fccba94098f7f93beaf26cc9d5)</span></li>
<li>kexi/kexiutils/KexiCommandLinkButton.h <span style="color: grey">(e5270eaeacd4e563742ca1120caaaad561f45d69)</span></li>
<li>kexi/kexiutils/KexiCommandLinkButton.cpp <span style="color: grey">(a1cff931012678b7c6dac09f4c70411c773db84e)</span></li>
<li>kexi/kexiutils/KexiHyperLinkHandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kexi/kexiutils/KexiHyperLinkHandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kexi/plugins/forms/CMakeLists.txt <span style="color: grey">(35142914417d12fba7d03078657690582d584612)</span></li>
<li>kexi/plugins/forms/kexidbfactory.cpp <span style="color: grey">(ff6146b7f2de008ad1bcce7b97f8007fed37710c)</span></li>
<li>kexi/plugins/forms/kexiformview.cpp <span style="color: grey">(dd6d31298955b65941c501f15f0b0f5749d7c4ae)</span></li>
<li>kexi/plugins/forms/widgets/kexidbautofield.cpp <span style="color: grey">(daa3c2ee730bd232a7ff656d6a3d00c2d87007cf)</span></li>
<li>kexi/plugins/forms/widgets/kexidbcommandlinkbutton.h <span style="color: grey">(39a68c8c43c912985992c389b88bd0c49aa7f703)</span></li>
<li>kexi/plugins/forms/widgets/kexidbcommandlinkbutton.cpp <span style="color: grey">(1592d2e0ba04a24cd856519fe4d988dd25eef5b9)</span></li>
<li>kexi/plugins/forms/widgets/kexipushbutton.h <span style="color: grey">(c3b9113f0839a704c0807e70afff9a5bdf232d50)</span></li>
<li>kexi/plugins/forms/widgets/kexipushbutton.cpp <span style="color: grey">(0cfe327c83f6abc4597826456bed6f6bbabe7dbd)</span></li>
<li>kexi/widget/CMakeLists.txt <span style="color: grey">(cf6be390218b4e620719acd4bc5e0c6fb19d1531)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108677/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>