> 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 > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
