Review Request 108677: Add support for hyperlinks in kexi buttons

Wed Feb 6 20:14:15 GMT 2013

> On Feb. 1, 2013, 11:36 p.m., Jarosław Staniek wrote:
> > kexi/kexiutils/KexiPushButton.cpp, line 84
> > <http://git.reviewboard.kde.org/r/108677/diff/2/?file=111331#file111331line84>
> >
> >     Shouldn't we add here:
> >     if (executable) { KRun::run(....); }
> 
> Oleg Kukharchuk wrote:
>     You think KRun::runUrl(url, type, q); not enough for that? Of course we should use defaultHyperlinkTool for that.
> 
> Jarosław Staniek wrote:
>     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?
>
> 
> Oleg Kukharchuk wrote:
>     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?

> 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

- Jarosław

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

On Feb. 1, 2013, 12:57 p.m., Oleg Kukharchuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108677/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2013, 12:57 p.m.)
> 
> 
> Review request for Calligra, Adam Pigg, Jarosław Staniek, and Dimitrios Tanis.
> 
> 
> Description
> -------
> 
> added support for hyperlinks in kexidbpushbutton and kexicommandlinkbutton
> 
> 
> Diffs
> -----
> 
>   kexi/kexiutils/CMakeLists.txt 19a3320b9138b54fdbbe1fee14dabb70cd1b9ba8 
>   kexi/kexiutils/KexiCommandLinkButton.h e5270eaeacd4e563742ca1120caaaad561f45d69 
>   kexi/kexiutils/KexiCommandLinkButton.cpp a1cff931012678b7c6dac09f4c70411c773db84e 
>   kexi/kexiutils/KexiPushButton.h PRE-CREATION 
>   kexi/kexiutils/KexiPushButton.cpp PRE-CREATION 
>   kexi/plugins/forms/CMakeLists.txt 35142914417d12fba7d03078657690582d584612 
>   kexi/plugins/forms/kexidbfactory.cpp f8bbab480e0ee92891617a3fe5e3bfbe51b78db8 
>   kexi/plugins/forms/kexiformview.cpp 3ffc0a8d9f9f3171873a569b28e44b712d747162 
>   kexi/plugins/forms/widgets/KexiDBPushButton.h PRE-CREATION 
>   kexi/plugins/forms/widgets/KexiDBPushButton.cpp PRE-CREATION 
>   kexi/plugins/forms/widgets/kexidbautofield.cpp daa3c2ee730bd232a7ff656d6a3d00c2d87007cf 
>   kexi/plugins/forms/widgets/kexidbcommandlinkbutton.h 39a68c8c43c912985992c389b88bd0c49aa7f703 
>   kexi/plugins/forms/widgets/kexidbcommandlinkbutton.cpp 1592d2e0ba04a24cd856519fe4d988dd25eef5b9 
>   kexi/plugins/forms/widgets/kexipushbutton.h c3b9113f0839a704c0807e70afff9a5bdf232d50 
>   kexi/plugins/forms/widgets/kexipushbutton.cpp 0cfe327c83f6abc4597826456bed6f6bbabe7dbd 
> 
> Diff: http://git.reviewboard.kde.org/r/108677/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Oleg Kukharchuk
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130206/bf9b0a86/attachment.htm>