> On Nov. 1, 2011, 3:07 p.m., Aaron J. Seigo wrote:
> > the wholesale copying of KOpenWithDialog is very unfortunate and needs to
> > be avoided if at *all* possible. why is it copied instead of used directly?
> >
> > the really big issue is the exec()'ing of the dialog, however.
> >
> > it would also be nice to be able to access individual launcher
> > configuration via the individual launchers' context menu.
> >
> > p.s. whitespace :)
>
> Craig Drummond wrote:
> I agree the copying is bad - but without changing kdelibs I knew of no
> other way. The reason it is copied is that in KOpenWithDialog you cannot
> remove the "Open with" label (or change its text), or the kurlrequester below
> it.
>
> Aaron J. Seigo wrote:
> the Open With label is settable by using the KOpenWithDialog( const
> KUrl::List& urls, const QString& text, const QString& value, QWidget *parent
> = 0 ); constructor, so it could be set to something sensible for this case.
> as forthe KUrlRequester ... is that so bad? it allows the user to define an
> arbitrary exec on disk, the ultimate "last resort".
Just tried KOpenWithDialog, and whilst you can set the text, and hide the
checkboxes - the text entered in the url combo *must* be an application, not a
desktop file. If you select a desktop file via the file selector you get the
following error:
"'XXX.desktop' not found, please type a valid program name."
So, KOpenWithDialog would need updating to resolve this.
> On Nov. 1, 2011, 3:07 p.m., Aaron J. Seigo wrote:
> > libs/taskmanager/taskactions.cpp, line 375
> > <http://git.reviewboard.kde.org/r/103007/diff/1/?file=39955#file39955line375>
> >
> > dialogs must not be exec()'d as they *freeze* the entire UI.. in this
> > case the whole plasma-desktop shell. i must be handled asynchronously.
>
> Craig Drummond wrote:
> Good point!
>
> Craig Drummond wrote:
> This is odd. I've tried running the dialog via "show()" and connecting to
> the "okClicked()" signal. However, the slot never gets called. If I change it
> to "exec()" the slot does get called...
>
> e.g. this does *not* work:
>
> KApplicationSelectorDialog *dlg=new
> KApplicationSelectorDialog(i18n("XXX"), 0L);
> connect(dlg, SIGNAL(okClicked()), this, SLOT(xxx()));
> dlg->show();
>
> ...but this does:
>
> KApplicationSelectorDialog *dlg=new
> KApplicationSelectorDialog(i18n("XXX"), 0L);
> connect(dlg, SIGNAL(okClicked()), this, SLOT(xxx()));
> dlg->exec();
>
> (Ignore the obvious memory leak)
>
> -----------------
>
> However, this seems to be a general issue with plasma dialogs. e.g. if
> you configure a colour for the clock dialog, plasma is frozen. Perhaps all
> plasma dialogs need to be run via another process?
>
> Aaron J. Seigo wrote:
> "e.g. this does *not* work"
>
> that is very, very odd. it would seem to imply that there is an event
> loop issue. there is no difference otherwise between exec() and show() (in
> fact, in fixing kcolorbutton i use show()).
>
> ah! perhaps i see the issue: LauncherProperties::run calls exec() on
> itself .. that creates an inner event loop around LauncherProperties. my
> guess is that trips it up. neither LauncherProperties nor
> KApplicationSelectorDialog should be exec()'d
>
> "(Ignore the obvious memory leak)"
>
> btw, easiest way to deal with this is to call
> dlg->setAttribute(Qt::WA_DeleteOnClose);
>
> "this seems to be a general issue with plasma dialog"
>
> we have this issue in very few places, and where it remains it is fixable
> .. i just fixed KColorButton right now in fact (thanks for pointing that one
> out)
We are looking in different places of the code. The exec I was talking about
was from the "Show A Launcher" action from the task menu.
I'll change the LauncherProperties to not use exec...
- Craig
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103007/#review7801
-----------------------------------------------------------
On Oct. 31, 2011, 8:42 p.m., Craig Drummond wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103007/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2011, 8:42 p.m.)
>
>
> Review request for Plasma.
>
>
> Description
> -------
>
> 1. If fail to automatically find launcher, then prompt user to select from
> installed applications.
> 2. Add a config page, so that manualy set launchers may be adjusted.
>
> (Part of IconTasks' taskmanager changes)
>
>
> Diffs
> -----
>
> libs/taskmanager/CMakeLists.txt 57f5f73
> libs/taskmanager/groupmanager.h acaa142
> libs/taskmanager/groupmanager.cpp 6e7ffa7
> libs/taskmanager/kapplicationselectordialog.h PRE-CREATION
> libs/taskmanager/kapplicationselectordialog.cpp PRE-CREATION
> libs/taskmanager/launcherconfig.h PRE-CREATION
> libs/taskmanager/launcherconfig.cpp PRE-CREATION
> libs/taskmanager/launcherconfig.ui PRE-CREATION
> libs/taskmanager/launcherproperties.h PRE-CREATION
> libs/taskmanager/launcherproperties.cpp PRE-CREATION
> libs/taskmanager/launcherproperties.ui PRE-CREATION
> libs/taskmanager/taskactions.cpp 0e6ba8e
> libs/taskmanager/taskitem.h 5de8478
> libs/taskmanager/taskitem.cpp 0a768e5
>
> Diff: http://git.reviewboard.kde.org/r/103007/diff/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Craig Drummond
>
>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
