On Sunday, November 14, 2010, Chani wrote: > > > breaks creation of new activities that aren't from templates :) > > > either give me a different method that plasmaapp can use, or revert > > > that line. > > > > whatever code is doing that is broken. that it worked at all was an > > accident of Corona::addContainment not sticking to the contract of > > immutability. > > > > what is the exact use case that is failing here? > > 1) > using addContainment to create a new empty containment for a new activity - > PlasmaApp::createActivity(pluginname) iirc.
createActivity should not be called when the Corona is locked. the only way i can see this becoming an issue is if an activity is created by a third application and we want plasma-desktop to react to that by adding a containt (or set of containments) for that activity when it is created in activity manager. personally i don't think that's a valid use case. there should be one place that activities are created and destroyed only: the desktop shell. > 2) using addContainment to create a new containment when a screen is > plugged in. this is a valid use case, at least for when Corona is UserImmutable. this is a bug in Activity::addContainment for not unlocking the Corona first. however, in the case of system immutability (kiosk), the Corona will refuse to unlock. which means that in a "fully" locked down system, adding a second screen will result in no desktop containment appearing for it. to be honest, that sounds about right. one solution would be to move the creation of screen/desktop related containments internal to Corona, e.g. by adding a new containmentForScreen method, sth like: Containment *containmentForScreen(int screen, int desktop = -1, const QString defaultPluginIfNonExistent = QString()); see attached diff. we probably also should add a new Kiosk restriction to add to this lot: http://techbase.kde.org/KDE_System_Administration/Kiosk/Keys#Plasma which should allow the restriction of the creation of new activities. this would allow one to have the Corona itself unlocked, but not create activities. this is a simple matter of documenting the key on that page and adding a check for it in plasma-desktop with: if (KAuthorized::authorize("plasma/create_new_activities")) { another option for a system administrator would be to add an entry in the system-wide plasma config like this: [General] immutability[$i]=2 in theory that would make it impossible to change the value of immutability to Mutable. i say in theory because while the value would be preserved between restarts, at runtime one would still be able to "Unlock widgets" in plasma- desktop. still, plasma code would need to be altered to check to see if that key is immutable and if it is to treat it as non-changeable. in fact, this is probably viewable as a bug right now, in fact, since it is, in pratice, not possible to make that config key immutable using kiosk. almost sounds like we need a "bool Corona::canChangeImmutability() const" method. if that was respected properly, then it would be possible to otherwise lock down the Corona while allowing it to do internal bookkeeping. thoughts? -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks
Index: corona.cpp =================================================================== --- corona.cpp (revision 1196287) +++ corona.cpp (working copy) @@ -33,13 +33,14 @@ #include <cmath> +#include <kaction.h> +#include <kactioncollection.h> #include <kdebug.h> #include <kglobal.h> #include <klocale.h> #include <kmimetype.h> -#include <kactioncollection.h> -#include <kaction.h> #include <kshortcutsdialog.h> +#include <kwindowsystem.h> #include "animator.h" #include "abstracttoolbox.h" @@ -192,7 +193,7 @@ //kDebug() << "Loading" << name << args << id; - if (pluginName.isEmpty()) { + if (pluginName.isEmpty() || pluginName == "default") { // default to the desktop containment pluginName = "desktop"; } @@ -583,6 +584,23 @@ return 0; } +Containment *Corona::containmentForScreen(int screen, int desktop, + const QString &defaultPluginIfNonExistent, const QVariantList &defaultArgs) +{ + Containment *containment = containmentForScreen(screen, desktop); + if (!containment && !defaultPluginIfNonExistent.isEmpty()) { + // screen requests are allowed to bypass immutability + if (screen >= 0 && screen < numScreens() && + desktop >= -1 && desktop < KWindowSystem::numberOfDesktops()) { + containment = d->addContainment(defaultPluginIfNonExistent, defaultArgs, 0, false); + if (containment) { + containment->setScreen(screen, desktop); + } + } + } + + return containment; +} QList<Containment*> Corona::containments() const { return d->containments; Index: corona.h =================================================================== --- corona.h (revision 1196283) +++ corona.h (working copy) @@ -96,7 +96,7 @@ Containment *addContainment(const QString &name, const QVariantList &args = QVariantList()); /** - * Returns the Containment, if any, for a given physical screen + * Returns the Containment, if any, for a given physical screen and desktop * * @param screen number of the physical screen to locate * @param desktop the virtual desktop) to locate; if < 0 then it will @@ -105,6 +105,20 @@ Containment *containmentForScreen(int screen, int desktop = -1) const; /** + * Returns the Containment for a given physical screen and desktop, creating one + * if none exists + * + * @param screen number of the physical screen to locate + * @param desktop the virtual desktop) to locate; if < 0 then it will + * simply return the first Containment associated with screen + * @param defaultPluginIfNonExistent the plugin to load by default; "null" is an empty + * Containment and "default" creates the default plugin + * @param defaultArgs optional arguments to pass in when creating a Containment if needed + */ + Containment *containmentForScreen(int screen, int desktop, + const QString &defaultPluginIfNonExistent, + const QVariantList &defaultArgs = QVariantList()); + /** * Adds a widget in the topleft quadrant in the scene. Widgets in the topleft quadrant are * normally never shown unless you specifically aim a view at it, which makes it ideal for * toplevel views etc.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel