Bug 141813

Created attachment 19712 [details]
xml where I can't delete labels or multiplicity

SVN commit 634394 by okellogg:

setName(): Remove special-casing code for when m_pName is non-NULL but strName
 is empty. This is a sensitive spot so I put the offending code in a comment
 for documentation.
BUG:141813


 M  +1 -0      ChangeLog  
 M  +2 -0      umbrello/associationwidget.cpp  


--- branches/KDE/3.5/kdesdk/umbrello/ChangeLog #634393:634394
@@ -7,6 +7,7 @@
 * Javascript Code Generation creates bad format methods (135540)
 * Crash when deleting the link between a package and a class (141602)
 * Ada95 Code Generation Errors for Aggregation (141644)
+* Unable to delete multiplicity information or lable from an association (141813)
 
 Version 1.5.61
 
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/associationwidget.cpp #634393:634394
@@ -327,11 +327,13 @@
             m_pName->setUMLObject(m_role[B].m_pWidget->getUMLObject());
     } else {
         m_pName->setText(strName);
+        /**** This code leads to bug 141813 (unable to delete label or multi)
         if (strName.isEmpty()) {
             m_pName->hide();
             m_pName = NULL;
             return;
         }
+         ********************************************************************/
     }
 
     // set attribute of UMLAssociation associated with this associationwidget

*** Bug 120699 has been marked as a duplicate of this bug. ***

SVN commit 651498 by okellogg:

It turns out that I was too quick closing this PR, here are some changes that
hopefully clear out the problem in its entirety.

AssociationWidget::cleanup(): Make public. Invoke removeAssocClassLine().

AssociationWidget destructor: Remove call to cleanup(), this is much too late
 to do that.  It is the caller's responsibility to call cleanup() prior to
 physically destructing an AssociationWidget.

AssociationWidget::setFloatingText(): New method factors common processing
 from setRoleName() and setMulti().

CCBUG:141813


 M  +35 -61    associationwidget.cpp  
 M  +20 -7     associationwidget.h  
 M  +2 -1      floatingtextwidget.cpp  
 M  +1 -0      umlview.cpp  


--- branches/KDE/3.5/kdesdk/umbrello/umbrello/associationwidget.cpp #651497:651498
@@ -127,8 +127,6 @@
 }
 
 AssociationWidget::~AssociationWidget() {
-    cleanup();
-    removeAssocClassLine();
 }
 
 AssociationWidget& AssociationWidget::operator=(AssociationWidget & Other) {
@@ -354,48 +352,50 @@
 
 }
 
-void AssociationWidget::setMulti(const QString &strMulti, Role_Type role) {
-    bool newLabel = false;
-    Text_Role tr = (role == A ? tr_MultiA : tr_MultiB);
+void AssociationWidget::setFloatingText(Uml::Text_Role tr,
+                                        const QString &text,
+                                        FloatingTextWidget* &ft) {
+    if (! FloatingTextWidget::isTextValid(text)) {
+        if (ft) {
+            // Remove preexisting FloatingTextWidget
+            m_pView->removeWidget(ft);  // physically deletes ft
+            ft = NULL;
+        }
+        return;
+    }
 
-    if(!m_role[role].m_pMulti) {
-        // Don't construct the FloatingTextWidget if the string is empty.
-        if (strMulti.isEmpty())
-            return;
-
+    bool newLabel = false;
+    if (ft == NULL) {
+        ft = new FloatingTextWidget(m_pView, tr, text);
+        ft->setLink(this);
+        m_pView->addWidget(ft);
         newLabel = true;
-        m_role[role].m_pMulti = new FloatingTextWidget(m_pView, tr, strMulti);
-        m_role[role].m_pMulti->setLink(this);
-        m_pView->addWidget(m_role[role].m_pMulti);
     } else {
-        if (m_role[role].m_pMulti->getText().isEmpty()) {
+        if (ft->getText().isEmpty()) {
             newLabel = true;
         }
-        m_role[role].m_pMulti->setText(strMulti);
-        if (strMulti.isEmpty()) {
-            m_role[role].m_pMulti->hide();
-            m_role[role].m_pMulti = NULL;
-            return;
-        }
+        ft->setText(text);
     }
 
-    m_role[role].m_pMulti->setActivated();
+    ft->setActivated();
 
     if (newLabel) {
         setTextPosition( tr );
     }
 
-    if(FloatingTextWidget::isTextValid(m_role[role].m_pMulti->getText()))
-        m_role[role].m_pMulti -> show();
-    else
-        m_role[role].m_pMulti -> hide();
+    ft->show();
+}
 
+void AssociationWidget::setMulti(const QString &strMulti, Role_Type role) {
+    Text_Role tr = (role == A ? tr_MultiA : tr_MultiB);
+
+    setFloatingText(tr, strMulti, m_role[role].m_pMulti);
+
     if (m_pObject && m_pObject->getBaseType() == ot_Association)
         getAssociation()->setMulti(strMulti, role);
 }
 
 void AssociationWidget::setRoleName (const QString &strRole, Role_Type role) {
-    bool newLabel = false;
     Association_Type type = getAssocType();
     //if the association is not supposed to have a Role FloatingTextWidget
     if (!AssocRules::allowRole(type))  {
@@ -403,44 +403,21 @@
     }
 
     Text_Role tr = (role == A ? tr_RoleAName : tr_RoleBName);
-    if(!m_role[role].m_pRole) {
-        // Don't construct the FloatingTextWidget if the string is empty.
-        if (strRole.isEmpty())
-            return;
-
-        newLabel = true;
-        m_role[role].m_pRole = new FloatingTextWidget(m_pView, tr, strRole);
-        m_role[role].m_pRole->setLink(this);
-        m_pView->addWidget(m_role[role].m_pRole);
+    setFloatingText(tr, strRole, m_role[role].m_pRole);
+    if (m_role[role].m_pRole) {
         Uml::Visibility vis = getVisibility(role);
-        m_role[role].m_pRole->setPreText(vis.toString(true));
-    } else {
-        if (m_role[role].m_pRole->getText().isEmpty()) {
-            newLabel = true;
+        if (FloatingTextWidget::isTextValid(m_role[role].m_pRole->getText())) {
+            m_role[role].m_pRole->setPreText(vis.toString(true));
+            //m_role[role].m_pRole->show();
+        } else {
+            m_role[role].m_pRole->setPreText("");
+            //m_role[role].m_pRole->hide();
         }
-        m_role[role].m_pRole->setText(strRole);
-        if (strRole.isEmpty()) {
-            m_role[role].m_pRole->hide();
-            m_role[role].m_pRole = NULL;
-            return;
-        }
     }
 
     // set attribute of UMLAssociation associated with this associationwidget
     if (m_pObject && m_pObject->getBaseType() == ot_Association)
         getAssociation()->setRoleName(strRole, role);
-    m_role[role].m_RoleName = strRole;
-
-    m_role[role].m_pRole->setActivated();
-
-    if (newLabel) {
-        setTextPosition( tr );
-    }
-
-    if(FloatingTextWidget::isTextValid(m_role[role].m_pRole->getText()))
-        m_role[role].m_pRole -> show();
-    else
-        m_role[role].m_pRole -> hide();
 }
 
 void AssociationWidget::setRoleDoc (const QString &doc, Role_Type role) {
@@ -783,6 +760,7 @@
     }
 
     m_LinePath.cleanup();
+    removeAssocClassLine();
 }
 
 void AssociationWidget::setUMLAssociation (UMLAssociation * assoc)
@@ -3188,10 +3166,6 @@
 
     // Initialize local members.
     // These are only used if we don't have a UMLAssociation attached.
-    m_role[A].m_Visibility = Uml::Visibility::Public;
-    m_role[B].m_Visibility = Uml::Visibility::Public;
-    m_role[A].m_Changeability = chg_Changeable;
-    m_role[B].m_Changeability = chg_Changeable;
     m_AssocType = Uml::at_Association;
     m_umldoc = UMLApp::app()->getDocument();
     m_LinePath.setAssociation( this );
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/associationwidget.h #651497:651498
@@ -626,17 +626,17 @@
     bool loadFromXMI( QDomElement & qElement, const UMLWidgetList& widgets,
                       const MessageWidgetList* pMessages = NULL);
 
+    /**
+     * Cleans up all the association's data in the related widgets.
+     */
+    void cleanup();
+
 private:
 
     /** set our internal umlAssociation */
     void setUMLAssociation (UMLAssociation * assoc);
 
     /**
-     * Cleans up all the association's data in the related widgets.
-     */
-    void cleanup();
-
-    /**
      * Merges/syncs the association widget data into UML object
      * representation.
      * CHECK: Can we get rid of this.
@@ -847,12 +847,25 @@
         Uml::Visibility m_Visibility;
         Uml::Changeability_Type m_Changeability;
         QString m_RoleDoc;
-        QString m_RoleName;
-        QString m_Multi;
 
     } m_role[2];
 
     /**
+     * Change, create, or delete the FloatingTextWidget indicated by the given Text_Role.
+     *
+     * @param tr    Text_Role of the FloatingTextWidget to change or create.
+     * @param text  Text string that controls the action:
+     *              If empty and ft is NULL then setFloatingText() is a no-op.
+     *              If empty and ft is non-NULL then the existing ft is deleted.
+     *              If non-empty and ft is NULL then a new FloatingTextWidget is created
+     *              and returned in ft with the text set.
+     *              If non-empty and ft is non-NULL then the existing ft text is modified.
+     * @param ft    Reference to the pointer to FloatingTextWidget to change or create.
+     *              On creation/deletion, the pointer value will be changed.
+     */
+    void setFloatingText(Uml::Text_Role tr, const QString &text, FloatingTextWidget* &ft);
+
+    /**
      * Called to tell the association that another association has added
      * a line to the region of one of its widgets. The widget is identified
      * by its role (A or B).
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/floatingtextwidget.cpp #651497:651498
@@ -202,15 +202,16 @@
                 break;
               default:
                 assoc->setName(QString::null);
+                m_pView->removeWidget(this);
                 break;
             }
         } else {
             MessageWidget *msg = dynamic_cast<MessageWidget*>(m_pLink);
             if (msg) {
                 msg->setName(QString::null);
+                m_pView->removeWidget(this);
             }
         }
-        m_pView->removeWidget(this);
         return;
     }
     if (m_pLink && m_Role != Uml::tr_Seq_Message && m_Role != Uml::tr_Seq_Message_Self) {
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umlview.cpp #651497:651498
@@ -1749,6 +1749,7 @@
 
     emit sigAssociationRemoved(pAssoc);
 
+    pAssoc->cleanup();
     m_AssociationList.remove(pAssoc); // will delete our association
     m_pDoc->setModified();
 }