Review Request 108677: Add support for hyperlinks in kexi buttons

Sun Jun 2 19:11:41 BST 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?
> 
> Jarosław Staniek 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?
>     
>     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
>

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

- Oleg

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

On May 29, 2013, 8 p.m., Oleg Kukharchuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108677/
> -----------------------------------------------------------
> 
> (Updated May 29, 2013, 8 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 943eae1f147c65fccba94098f7f93beaf26cc9d5 
>   kexi/kexiutils/KexiCommandLinkButton.h e5270eaeacd4e563742ca1120caaaad561f45d69 
>   kexi/kexiutils/KexiCommandLinkButton.cpp a1cff931012678b7c6dac09f4c70411c773db84e 
>   kexi/kexiutils/KexiHyperLinkHandler.h PRE-CREATION 
>   kexi/kexiutils/KexiHyperLinkHandler.cpp PRE-CREATION 
>   kexi/plugins/forms/CMakeLists.txt 35142914417d12fba7d03078657690582d584612 
>   kexi/plugins/forms/kexidbfactory.cpp ff6146b7f2de008ad1bcce7b97f8007fed37710c 
>   kexi/plugins/forms/kexiformview.cpp dd6d31298955b65941c501f15f0b0f5749d7c4ae 
>   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 
>   kexi/widget/CMakeLists.txt cf6be390218b4e620719acd4bc5e0c6fb19d1531 
> 
> 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/20130602/c37cc9cd/attachment.htm>