[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|REOPENED|RESOLVED

--- Comment #43 from Ralf Habacker  ---
Remaining issue has been reported with bug 374879.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Attachment #102921|0   |1
is obsolete||

--- Comment #42 from Ralf Habacker  ---
Comment on attachment 102921
  --> https://bugs.kde.org/attachment.cgi?id=102921
Fix NULL pointer derefences found by Coverity - CIDs 173354 and 173353.

Patch with coverity fixes has been applied to git master. Many thanks for
contributing.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #41 from Ralf Habacker  ---
Git commit 9ed47597b5506956e1ca9f4bced7ece3efe12d3e by Ralf Habacker, on behalf
of Carsten Clever.
Committed on 21/12/2016 at 14:15.
Pushed by habacker into branch 'master'.

Fix coverity check CID 173354 and 173353: NULL pointer derefences
Related: bug 340646

M  +5-3umbrello/cmds/widget/cmdremovewidget.cpp
M  +3-2umbrello/umlscene.cpp

https://commits.kde.org/umbrello/9ed47597b5506956e1ca9f4bced7ece3efe12d3e

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #40 from clush...@gmx.net ---
Created attachment 102921
  --> https://bugs.kde.org/attachment.cgi?id=102921=edit
Fix NULL pointer derefences found by Coverity - CIDs 173354 and 173353.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #39 from Ralf Habacker  ---
At https://mail.kde.org/pipermail/umbrello-devel/2016-December/020658.html it
looks that coverity detected implementation issues with your code (see CID
173354 and 173353). It would be nice to have a fix the for the mentioned
issues.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #38 from clush...@gmx.net ---
(In reply to Ralf Habacker from comment #36)

This is not straight forward to fix because for Ports, the parent widget is not
set at the required point in time. A PortWidget first creates its parent
PinPortBase, and afterwards sets the parent widget. But PinPortBase c'tor calls
its init() function which would then try to place the widget relative to the
parent. The code should be refactored first to set the parent widget as early
as possible.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #37 from Ralf Habacker  ---
Git commit f389d9d163d5755fb57db9a834e28d4e3b8bd9fa by Ralf Habacker.
Committed on 08/12/2016 at 20:06.
Pushed by habacker into branch 'master'.

Local fix for using deprecated Qt5 method QGraphicsScene::itemAt(QPointF).

M  +9-0umbrello/umlscene.h

https://commits.kde.org/umbrello/f389d9d163d5755fb57db9a834e28d4e3b8bd9fa

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #36 from Ralf Habacker  ---
Comparing the new implementation with the old one from 2.20 shows a little
remaining issue:

 In the old implementation the position of the pin/port been added depends on
the current mouse position. If the mouse is in the upper half of the related
widget, the pin/port is added on top of the widget and at the horizontal mouse
position. If the mouse is in the lower half of the widget the pin/port is added
on the bottom and at the horizontal mouse position.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #34 from Ralf Habacker  ---
I just tried to connect a pin on an activity diagram with another activity. The
result is that one end of the connection ends on the "another activity", the
other end does *not* ends on the pin. It ends somewhere in empty diagram space. 
My guess it that the connection end point calculation seems to be wrong now.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Blocks||373249


Referenced Bugs:

https://bugs.kde.org/show_bug.cgi?id=373249
[Bug 373249] Ports on components are not aligned at component border
-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #33 from clush...@gmx.net ---
Thank you for accepting and committing the changes. I can continue to work on
this topic including refactorings. For actual fixes of differences between
Components and Activities, I would need further insight that might come along
with working on the code base.
For simplicity, I suggest we continue to use this ticket for now.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #32 from Ralf Habacker  ---
(In reply to Ralf Habacker from comment #31)
> From a internet research I got the impression that ports are used used with
> Components (https://en.wikipedia.org/wiki/Component_diagram) 

Looking at this page and having your updated implementation of PinPortWidget it
looks now possible to place the ports on the component border (---[]---)
instead of outside ( []__)

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #31 from Ralf Habacker  ---
(In reply to clushkou from comment #20)
> I tried to fulfill your request in comment #17, but do not understand how to
> achieve this.
I applied your first approach to git master. Thanks for that contribution.

>  If I simply move the calls to ComponentWidget, I get a crash
> on undo because restoring of child element does not work.

It looks that this would require more refactoring :-( See also my comment
below.

> In addition, if the code needs to be added to ComponentWidget, the same
> adaption is needed in all widgets that allow child Pins / Ports.

>From a internet research I got the impression that ports are used used with
Components (https://en.wikipedia.org/wiki/Component_diagram) and pins with
activities (https://de.wikipedia.org/wiki/Aktion_(UML)).

The load/save implementation differs also between pins and ports in that way:
activities and pins are not backed up by a related uml model objects, they are
only widgets. The relation from the pin widget to the activity widget is
handled with the  tag attribute "widgetaid". The same is true for
the relation port widget to component widget.

Currently I do not have any idea how the refactoring could be done. With that
we have two choices:
1. leave this bug open as reminder for that open task or
2. close this bug because the initial crash has been fixed and a basic
refactoring has been implemented and to open a new bug describing the left
issue as a reminder.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Attachment #102602|0   |1
is obsolete||

--- Comment #30 from Ralf Habacker  ---
Created attachment 102603
  --> https://bugs.kde.org/attachment.cgi?id=102603=edit
Another patch proposal. CompoentWidget::loadFromXmi and ::saveToXmi adapted,
but crashing. (rebased)

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #29 from Ralf Habacker  ---
Created attachment 102602
  --> https://bugs.kde.org/attachment.cgi?id=102602=edit
Turn PinPortBase into child widget.

BUG:371990

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Attachment #102601|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #27 from Ralf Habacker  ---
Created attachment 102601
  --> https://bugs.kde.org/attachment.cgi?id=102601=edit
Turn PinPortBase into real child widget.

CCBUG:371990

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Attachment #102527|0   |1
is obsolete||

--- Comment #28 from Ralf Habacker  ---
Comment on attachment 102527
  --> https://bugs.kde.org/attachment.cgi?id=102527
Another patch proposal. CompoentWidget::loadFromXmi and ::saveToXmi adapted,
but crashing.

Rebased last patch

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Attachment #102312|0   |1
is obsolete||

--- Comment #26 from Ralf Habacker  ---
Comment on attachment 102312
  --> https://bugs.kde.org/attachment.cgi?id=102312
Turn PinPortBase into real child widget.

committed to master.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #25 from Ralf Habacker  ---
Git commit 703b25a024380dcb81895c9700c3a8d4d72cfb74 by Ralf Habacker.
Committed on 03/12/2016 at 17:00.
Pushed by habacker into branch 'master'.

Turn PinPortBase into real child widget.

M  +17   -1umbrello/cmds/widget/cmdremovewidget.cpp
M  +2-1umbrello/cmds/widget/cmdremovewidget.h
M  +15   -44   umbrello/umlscene.cpp
M  +126  -127  umbrello/umlwidgets/pinportbase.cpp
M  +26   -11   umbrello/umlwidgets/pinportbase.h
M  +3-12   umbrello/umlwidgets/pinwidget.cpp
M  +0-1umbrello/umlwidgets/pinwidget.h
M  +3-15   umbrello/umlwidgets/portwidget.cpp
M  +1-2umbrello/umlwidgets/portwidget.h
M  +20   -4umbrello/umlwidgets/umlwidget.cpp
M  +1-0umbrello/umlwidgets/umlwidget.h
M  +8-8umbrello/umlwidgets/widget_factory.cpp

https://commits.kde.org/umbrello/703b25a024380dcb81895c9700c3a8d4d72cfb74

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Simon Andric]

https://bugs.kde.org/show_bug.cgi?id=371990

Simon Andric  changed:

   What|Removed |Added

 CC||simonandr...@gmail.com

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #24 from Ralf Habacker  ---
(In reply to Ralf Habacker from comment #22)

>... The in-memory
> representation changes this behavior by adding a direct association former
> with m_pow and now this QGraphicsItem child api.

This is used by the undo framework, but not by the copy/paste and load/save
framework. It would be nice to have a way to keep handling of port widgets in
sync in all mentioned frameworks.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #23 from Ralf Habacker  ---
(In reply to Ralf Habacker from comment #22)
> I see. For the record: umbrello uses saveToXMI and loadFromXMI() to handle
> xmi file save/load, object (de)serialisation on copy/paste between same and
> other umbrello instances and undo framework so it would be nice to hold
> related stuff together and to implement stuff in the same way if possible to
> reduce req
.. required support efforts and further extensions.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #22 from Ralf Habacker  ---
(In reply to clushkou from comment #20)
> I tried to fulfill your request in comment #17, but do not understand how to
> achieve this. If I simply move the calls to ComponentWidget, I get a crash
> on undo because restoring of child element does not work.
> In addition, if the code needs to be added to ComponentWidget, the same
> adaption is needed in all widgets that allow child Pins / Ports.

I see. For the record: umbrello uses saveToXMI and loadFromXMI() to handle xmi
file save/load, object (de)serialisation on copy/paste between same and other
umbrello instances and undo framework so it would be nice to hold related stuff
together and to implement stuff in the same way if possible to reduce req

What I did: I created a component widget with ports and saved it into a xmi
file. Inspecting this file shows that the component widget and the pin widgets
does not have any direct association. The association is performed by the
related  UML:Component which has UML:Port as child. The in-memory
representation changes this behavior by adding a direct association former with
m_pow and now this QGraphicsItem child api.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #21 from clush...@gmx.net ---
Created attachment 102527
  --> https://bugs.kde.org/attachment.cgi?id=102527=edit
Another patch proposal. CompoentWidget::loadFromXmi and ::saveToXmi adapted,
but crashing.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #20 from clush...@gmx.net ---
I tried to fulfill your request in comment #17, but do not understand how to
achieve this. If I simply move the calls to ComponentWidget, I get a crash on
undo because restoring of child element does not work.
In addition, if the code needs to be added to ComponentWidget, the same
adaption is needed in all widgets that allow child Pins / Ports.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Attachment #102311|0   |1
is obsolete||

--- Comment #19 from Ralf Habacker  ---
Comment on attachment 102311
  --> https://bugs.kde.org/attachment.cgi?id=102311
Restrict write access to UMLScene::widgetList to track NULL-pointer list
entries.

Applied to Applications/16.12 branch with CCBUG: key instead of BUG: to not
close the bug.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #18 from Ralf Habacker  ---
Git commit c7250ebcb0886f47f4e694a6b4d2ea369adc0a89 by Ralf Habacker, on behalf
of Carsten Clever.
Committed on 26/11/2016 at 22:07.
Pushed by habacker into branch 'Applications/16.12'.

Restrict write access to UMLScene::widgetList to track NULL-pointer list
entries.

M  +1-1umbrello/clipboard/umlclipboard.cpp
M  +1-1umbrello/clipboard/umlclipboard.h
M  +1-1umbrello/cmds/widget/cmdbasewidgetcommand.cpp
M  +2-2umbrello/codeimport/import_utils.cpp
M  +1-1umbrello/toolbarstatemessages.cpp
M  +12   -6umbrello/umlscene.cpp
M  +4-1umbrello/umlscene.h
M  +2-2umbrello/umlwidgets/combinedfragmentwidget.cpp

https://commits.kde.org/umbrello/c7250ebcb0886f47f4e694a6b4d2ea369adc0a89

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #17 from Ralf Habacker  ---
Comment on attachment 102312
  --> https://bugs.kde.org/attachment.cgi?id=102312
Turn PinPortBase into real child widget.

A quick look shows that the following code 

+foreach(QGraphicsItem* item, widget->childItems()) {
+UMLWidget* child = dynamic_cast(item);
+QDomDocument doc;
+m_children.append(doc.createElement(QLatin1String("child")));
+child->saveToXMI(doc, m_children.back());
+}
+

needs to goto into ComponentWidget::saveToXMI()

and this
+foreach(QDomElement childElement, m_children) {
+widgetElement = childElement.firstChild().toElement();
+widget = umlScene->loadWidgetFromXMI(widgetElement);
+if (0 != widget) {
+addWidgetToScene(widget);
+}
+}

into ComponentWidget::loadToXMI() to support also saving to and loading from
file. 

If this is fixed I'm going to test this implementation. Thanks for
contribution.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #16 from clush...@gmx.net ---
I could reproduce the crash. It's due to remove undo not restoring the child
widgets. Subsequent undo commands assume the child widget exists and access a
deleted widget.
I've splitted and updated the patch to fix this issue as well.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

clush...@gmx.net changed:

   What|Removed |Added

 Attachment #102196|0   |1
is obsolete||

--- Comment #15 from clush...@gmx.net ---
Created attachment 102312
  --> https://bugs.kde.org/attachment.cgi?id=102312=edit
Turn PinPortBase into real child widget.

(2/2) PinPortBase as child widget: Turn PinPortBase into real child widget.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #14 from clush...@gmx.net ---
Created attachment 102311
  --> https://bugs.kde.org/attachment.cgi?id=102311=edit
Restrict write access to UMLScene::widgetList to track NULL-pointer list
entries.

(1/2) PinPortBase as child widget: Restrict write access to
UMLScene::widgetList to track NULL-pointer list entries.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #13 from Ralf Habacker  ---
Comment on attachment 102196
  --> https://bugs.kde.org/attachment.cgi?id=102196
Turn PinPortBase into child item

I checked if this it works using the following use cases: 
1. Add component diagram
2. Add component
3. add two pins
4. save file
5. load file -> okay 
6. delete second pin
7. delete first pin
8. undo both delete -> okay
9 set different color for component and each pin
10. save and reload -> okay

Looks good :-) 

11. delete second ping
12. delete component
13. undo component delete
14. undo -> regardless of number of undo the first pin is not restored and need
to be readded manual (may be a different bug)

Running after creating a new file
1. Add component diagram
2. Add component
3. add two pins
4. delete second ping
5. delete component
6. undo component delete
7. repeated undo -> I get Q_ASSERT in CmdBaseWidgetCommand::widget()

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Resolution|FIXED   |---
 Status|RESOLVED|REOPENED

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Attachment #102037|0   |1
is obsolete||

--- Comment #12 from Ralf Habacker  ---
Comment on attachment 102037
  --> https://bugs.kde.org/attachment.cgi?id=102037
Patch to fix crash: Remove PinPortBase objects when owner is removed - rebased

applied to Application/16.08

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #11 from clush...@gmx.net ---
Created attachment 102196
  --> https://bugs.kde.org/attachment.cgi?id=102196=edit
Turn PinPortBase into child item

Attached patch cleans up m_pOw in PinPortBase, and fixes all issues arising due
to this change.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Resolution|--- |FIXED
  Latest Commit||http://commits.kde.org/umbr
   ||ello/536e52fe6f71145cdcee16
   ||b5833f1c73b64e61fa
 Status|CONFIRMED   |RESOLVED
   Version Fixed In||2.20.3 (KDE Applications
   ||16.08.3)

--- Comment #10 from Ralf Habacker  ---
Git commit 536e52fe6f71145cdcee16b5833f1c73b64e61fa by Ralf Habacker.
Committed on 08/11/2016 at 07:39.
Pushed by habacker into branch 'Applications/16.08'.

Fix 'Crash when removing pin widgets after owning component has been removed
before'.

The fix from clush...@gmx.net is designed to remove PinPortBase objects when
owner is remove.
FIXED-IN:2.20.3 (KDE Applications 16.08.3)

M  +41   -0umbrello/umlscene.cpp
M  +6-0umbrello/umlscene.h
M  +32   -1umbrello/umlwidgets/pinportbase.cpp
M  +6-1umbrello/umlwidgets/pinportbase.h
M  +4-4umbrello/umlwidgets/portwidget.cpp

http://commits.kde.org/umbrello/536e52fe6f71145cdcee16b5833f1c73b64e61fa

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Attachment #101978|0   |1
is obsolete||

--- Comment #9 from Ralf Habacker  ---
Comment on attachment 101978
  --> https://bugs.kde.org/attachment.cgi?id=101978
Patch to fix crash: Remove PinPortBase objects when owner is removed.

superseeded by attachment 102037

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #8 from clush...@gmx.net ---
Created attachment 102037
  --> https://bugs.kde.org/attachment.cgi?id=102037=edit
Patch to fix crash: Remove PinPortBase objects when owner is removed - rebased

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #7 from clush...@gmx.net ---
Thank's for commit the first part already. I've rebased and retested the fix
for the crash, and found another issue when selecting both component and
attached pin. That's fixed as well with the new patch.
I tried the suggested child/parent cleanup, but encounter several problems with
this. Briefly, the PinPortBase is a subclass of UMLWidget, but this class is
meant to represent root graphic items. The PinPortBase would be a child widget,
and needs to behave differently, e.g. when moving the parent component. I'll
continue to work on this.

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Attachment #102013|0   |1
is obsolete||

--- Comment #6 from Ralf Habacker  ---
Comment on attachment 102013
  --> https://bugs.kde.org/attachment.cgi?id=102013
Enforce valid baseType

Thanks for your contribution which is a good addition. You made m_baseType
private to force clients to use the wrapper. 

I applied that fix into umbrello git repo which requires that you rebase your
patch against umbrello-master. 
Because this is a crash bug I would apply the fix to Applications/16.08 to get
it into next release 2.20.3. Unfortunally 2.20.3 will be released at November
10, so there is not much time left. 
If you have problems to update your patch using the mentioned QGraphicsItems
proposal in that time frame, I will use the recent (and rebased) patch for that
release. The QGraphicsItem based fix would then be in the next release series
2.21 (Applications/16.12 branch)

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #5 from Ralf Habacker  ---
Git commit 2ed4f7baedb1766452a5b08ffaf882ab09dcdbcf by Ralf Habacker, on behalf
of Carsten Clever.
Committed on 04/11/2016 at 07:42.
Pushed by habacker into branch 'Applications/16.08'.

Add wrapper for WidgetBase::m_baseType and enforce valid range.

Reviewed by Ralf Habacker.

M  +4-4umbrello/umlwidgets/classifierwidget.cpp
M  +2-1umbrello/umlwidgets/pinportbase.cpp
M  +1-0umbrello/umlwidgets/portwidget.cpp
M  +4-4umbrello/umlwidgets/umlwidget.cpp
M  +12   -0umbrello/umlwidgets/widgetbase.cpp
M  +7-2umbrello/umlwidgets/widgetbase.h

http://commits.kde.org/umbrello/2ed4f7baedb1766452a5b08ffaf882ab09dcdbcf

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #4 from clush...@gmx.net ---
Created attachment 102013
  --> https://bugs.kde.org/attachment.cgi?id=102013=edit
Enforce valid baseType

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #3 from clush...@gmx.net ---
Regarding 1.: I'm not sure how much effort it will be, but it's definitely
better than the current approach. I'll give it a try, but might take some days.
Regarding 2.: You're right. I've created a separate patch for this improvement
to shorten the remaining changes. See new attachment.
Regarding 3.: Yes, I've encountered this too. Delete component with pins cannot
be undone, pins don't appear back. I think a proper solution might be to extend
the cmdremovewidget to maintain list of child widgets in addition to the parent
widget. What do you think?

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 Status|UNCONFIRMED |CONFIRMED
 Ever confirmed|0   |1

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[Ralf Habacker]

https://bugs.kde.org/show_bug.cgi?id=371990

Ralf Habacker  changed:

   What|Removed |Added

 CC||ralf.habac...@freenet.de

--- Comment #2 from Ralf Habacker  ---
Comment on attachment 101978
  --> https://bugs.kde.org/attachment.cgi?id=101978
Patch to fix crash: Remove PinPortBase objects when owner is removed.

I can confirm that the patch fixes the crash. Looking at the patch I have a few
questions/notes:
1. The owner management of the PinWidgets is done by the private member m_pOw.
UMLWidgets is derived from QGraphicsItem which also has a owner/parent
management with parentItem(). In principal it looks possible to use that
instead of m_pOw. Any idea how much the related effort would be to fix the
issue that way ? 
2. The usage of Q_ASSERT(m_baseType >= wt_UMLWidget && m_baseType <= wt_Port);
is good, but also introduces in further development to miss some added enum
values.  It would be better to introduce enum constants for the upper and lower
limit and to check against them.
3. Redoing has issues: I removed one pin, then the component and tried to redo
both with results into incomplete redo.
4. For detailed review there is reviewboard.kde.org

-- 
You are receiving this mail because:
You are watching all bug changes.

[umbrello] [Bug 371990] Crash when removing pin widgets after owning component has been removed before

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=371990

--- Comment #1 from clush...@gmx.net ---
Created attachment 101978
  --> https://bugs.kde.org/attachment.cgi?id=101978=edit
Patch to fix crash: Remove PinPortBase objects when owner is removed.

-- 
You are receiving this mail because:
You are watching all bug changes.