* prevent removing active containments (brute force mechanism: just ignore the request totally)

* select the correct containment for removal when zoomed out (will have isses on multi-screen; see FIXME)
* move containment destruction logic out of Corona; it really doesn't belong there
* remove the hack to not set focus on a containment when clicked as that doesn't seem to reliably trigger it for me here and it doesn't address the issue of a new containment still messing things up, so we really ought to have a generic solution instead of a slightly ugly hack

BUG:id=165670

svn path=/trunk/KDE/kdebase/workspace/libs/plasma/; revision=828906

applet.cpp

@@ -301,13 +301,19 @@ KConfigGroup Applet::globalConfig() const
 
 void Applet::destroy()
 {
-    if (d->transient) {
+    if (immutability() != Mutable || d->transient) {
         return; //don't double delete
     }
+
     d->transient = true;
-    connect(Animator::self(), SIGNAL(animationFinished(QGraphicsItem*,Plasma::Animator::Animation)),
-                        this, SLOT(appletAnimationComplete(QGraphicsItem*,Plasma::Animator::Animation)));
-    Animator::self()->animateItem(this, Animator::DisappearAnimation);
+
+    if (isContainment()) {
+        d->cleanUpAndDelete();
+    } else {
+        connect(Animator::self(), SIGNAL(animationFinished(QGraphicsItem*,Plasma::Animator::Animation)),
+                this, SLOT(appletAnimationComplete(QGraphicsItem*,Plasma::Animator::Animation)));
+        Animator::self()->animateItem(this, Animator::DisappearAnimation);
+    }
 }
 
 void AppletPrivate::appletAnimationComplete(QGraphicsItem *item, Plasma::Animator::Animation anim)
@@ -319,6 +325,27 @@ void AppletPrivate::appletAnimationComplete(QGraphicsItem *item, Plasma::Animato
     cleanUpAndDelete();
 }
 
+void AppletPrivate::selectItemToDestroy()
+{
+    //FIXME: this will not work nicely with multiple screens and being zoomed out!
+    if (q->isContainment() &&
+        q->view() && q->view()->transform().isScaling() &&
+        q->scene()->focusItem() != q) {
+        QGraphicsItem *focus = q->scene()->focusItem();
+
+        if (focus) {
+            Containment *toDestroy = dynamic_cast<Containment*>(focus->topLevelItem());
+
+            if (toDestroy) {
+                toDestroy->destroy();
+                return;
+            }
+        }
+    }
+
+    q->destroy();
+}
+
 void AppletPrivate::cleanUpAndDelete()
 {
     //kDebug() << "???????????????? DESTROYING APPLET" << name() << " ???????????????????????????";
@@ -736,7 +763,7 @@ void Applet::flushPendingConstraintsEvents()
         } else {
             closeApplet->setShortcut(QKeySequence("ctrl+r"));
         }
-        connect(closeApplet, SIGNAL(triggered(bool)), this, SLOT(destroy()));
+        connect(closeApplet, SIGNAL(triggered(bool)), this, SLOT(selectItemToDestroy()));
         d->actions.addAction("remove", closeApplet);
     }
 
@@ -1133,19 +1160,15 @@ void Applet::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
 
 void Applet::mousePressEvent(QGraphicsSceneMouseEvent *event)
 {
-    if (!isContainment() || (view() && !view()->transform().isScaling())) {
-        setFocus(Qt::MouseFocusReason);
-    }
+    setFocus(Qt::MouseFocusReason);
     QGraphicsWidget::mousePressEvent(event);
 }
 
 void Applet::focusInEvent(QFocusEvent * event)
 {
-    if (containment()) {
-        containment()->d->focusApplet(this);
-        //XXX if we are a containment we'll attempt to focus ourself
-        //which should be harmless
+    if (!isContainment() && containment()) {
         //focusing an applet may trigger this event again, but we won't be here more than twice
+        containment()->d->focusApplet(this);
     }
 
     QGraphicsWidget::focusInEvent(event);

applet.h

@@ -748,6 +748,7 @@ class PLASMA_EXPORT Applet : public QGraphicsWidget
         Q_PRIVATE_SLOT(d, void checkImmutability())
         Q_PRIVATE_SLOT(d, void themeChanged())
         Q_PRIVATE_SLOT(d, void appletAnimationComplete(QGraphicsItem *item, Plasma::Animator::Animation anim))
+        Q_PRIVATE_SLOT(d, void selectItemToDestroy())
 
         /**
          * Reimplemented from QGraphicsItem

applet_p.h

@@ -62,6 +62,7 @@ public:
     void themeChanged();
     void resetConfigurationObject();
     void appletAnimationComplete(QGraphicsItem *item, Plasma::Animator::Animation anim);
+    void selectItemToDestroy();
     void setFocus();
     void cleanUpAndDelete();
 

containment.cpp

@@ -595,7 +595,7 @@ Applet::List Containment::applets() const
 void Containment::setScreen(int screen)
 {
     // screen of -1 means no associated screen.
-    if (containmentType() == DesktopContainment || containmentType() >= CustomContainment) {
+    if (d->type == DesktopContainment || d->type == CustomContainment) {
 #ifndef Q_OS_WIN
         // we want to listen to changes in work area if our screen changes
         if (d->screen < 0 && screen > -1) {
@@ -1029,11 +1029,19 @@ void Containment::destroy()
     }
 
     if (isContainment()) {
+        //don't remove a desktop that's in use
+        //FIXME allow removal of containments for screens that don't currently exist
+        if (d->type != PanelContainment && d->type != CustomPanelContainment &&
+            (d->screen != -1 || d->screen >= QApplication::desktop()->numScreens())) {
+            kDebug() << (QObject*)this << "containment has a screen number?" << d->screen;
+            return;
+        }
+
         //FIXME maybe that %1 should be the containment type not the name
         if (KMessageBox::warningContinueCancel(view(), i18n("Do you really want to remove this %1?", name()),
                     i18n("Remove %1", name()), KStandardGuiItem::remove()) == KMessageBox::Continue ) {
-            clearApplets();
-            corona()->destroyContainment(this);
+            //clearApplets();
+            Applet::destroy();
         }
     } else {
         Applet::destroy();

corona.cpp

@@ -99,6 +99,7 @@ public:
 
         if (index > -1) {
             containments.removeAt(index);
+            q->requestConfigSync();
         }
     }
 
@@ -149,7 +150,7 @@ public:
         QObject::connect(containment, SIGNAL(configNeedsSaving()), q, SLOT(requestConfigSync()));
         QObject::connect(containment, SIGNAL(releaseVisualFocus()), q, SIGNAL(releaseVisualFocus()));
         QObject::connect(containment, SIGNAL(screenChanged(int,int,Plasma::Containment*)),
-                q, SIGNAL(screenOwnerChanged(int,int,Plasma::Containment*)));
+                         q, SIGNAL(screenOwnerChanged(int,int,Plasma::Containment*)));
 
         if (!delayedInit) {
             emit q->containmentAdded(containment);
@@ -331,25 +332,6 @@ Containment* Corona::addContainment(const QString& name, const QVariantList& arg
     return c;
 }
 
-void Corona::destroyContainment(Containment *c)
-{
-    if (!d->containments.contains(c)) {
-        return;
-    }
-
-    //don't remove a desktop that's in use
-    //FIXME allow removal of containments for screens that don't currently exist
-    if (c->containmentType() != Containment::PanelContainment && c->screen() != -1) {
-        return;
-    }
-
-    d->containments.removeAll(c);
-    removeItem(c);
-    c->config().deleteGroup();
-    c->deleteLater();
-    requestConfigSync();
-}
-
 void Corona::loadDefaultLayout()
 {
 }

corona.h

@@ -119,11 +119,6 @@ public Q_SLOTS:
      */
     void saveLayout(const QString &config = QString()) const;
 
-    /**
-     * Removes a given containment from the corona
-     */
-    void destroyContainment(Containment *containment);
-
     /**
      * @return The type of immutability of this Corona
      */