Bug 510485 - Wrong parameter in GFDragConnectionTool#continueConnection()
Summary: Wrong parameter in GFDragConnectionTool#continueConnection()
Status: ASSIGNED
Alias: None
Product: Graphiti
Classification: Modeling
Component: Core (show other bugs)
Version: 0.13.0   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-16 02:51 EST by Daniel Hoeh CLA
Modified: 2017-04-28 10:01 EDT (History)
1 user (show)

See Also:


Attachments
Modifed tutorial class (3.92 KB, text/x-java-source)
2017-01-20 10:39 EST, Michael Wenz CLA
no flags Details
Modified diagram (11.63 KB, application/octet-stream)
2017-01-20 10:39 EST, Michael Wenz CLA
no flags Details
Modified diagram with ChopboxAnchor and only one ContainerShape (10.29 KB, application/octet-stream)
2017-01-31 09:31 EST, Daniel Hoeh CLA
no flags Details
Corrected diagram (10.22 KB, application/octet-stream)
2017-01-31 09:36 EST, Daniel Hoeh CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Hoeh CLA 2017-01-16 02:51:11 EST
In method GFDragConnectionTool#continueConnection(), there are the parameters targetEditPart and targetTargetEditPart, where targetEditPart means the edit part of the connection source anchor and targetTargetEditPart means the edit part under the current drag point of the connection target, if I understand it correctly.

The next two lines actually should reference connection source and target but both refer to targetEditPart:
setConnectionSource(targetEditPart);
lockTargetEditPart(targetEditPart);

This causes a wrong behavior for example if a connection feature's #canCreate returns true when the drag point is not over an anchor - in that case, the visual feedback shows a "cannot connect" symbol although the further processing enables the connection - what in my case is the desired behavior.

I think the problem can be fixed by changing the first two lines of that method to:
setConnectionSource(targetEditPart);
lockTargetEditPart(targetTargetEditPart);
Comment 1 Michael Wenz CLA 2017-01-16 04:19:50 EST
This looks reasonable at first glance, but I will need to have a closer look.
Comment 2 Michael Wenz CLA 2017-01-20 10:38:13 EST
Daniel,

I just revisited that, are you sure that this does not work? I tried to reproduce with the tutorial, so I:
1) Modified an existing diagram and removed the ChopboxAnchors from the EClass shapes
2) Modified the tuturial class TutorialCreateEReferenceFeature so that it accepts to create on any EClass target shape

That worked fine for me, connections can now be created from the extra anchor shape of an EClass to any area of a target EClass shape.

When I change the lines you recommend, the only effect I see is that the highlighting the target does not work correctly any more.

Could you double check? I attach the modified diagram and the modified tutorial class here.

Thanks,
Michael
Comment 3 Michael Wenz CLA 2017-01-20 10:39:25 EST
Created attachment 266378 [details]
Modifed tutorial class
Comment 4 Michael Wenz CLA 2017-01-20 10:39:47 EST
Created attachment 266379 [details]
Modified diagram
Comment 5 Daniel Hoeh CLA 2017-01-20 10:56:29 EST
Michael,
thank you very much for your work. Unfortunately I'm leaving for vacation now, I'm back on Jan 30. In that week, I'll check what you said.
Cheers
Daniel
Comment 6 Daniel Hoeh CLA 2017-01-31 09:30:04 EST
Michael, we've done further investigations and in fact it needs more conditions to reproduce the problem:

1) The problem ONLY occurs when the connection is being started from a context button with drag&drop enabled. Such a button is added to the button pad from the tutorial's ToolBehaviorProvider in case the mouse-over PictogramElement has a ChopboxAnchor, i.e. your ToolBehaviorProvider should contain the tutorial code
...
        } else if (pe instanceof AnchorContainer) {
            // assume, that our shapes always have chopbox anchors
            anchor = Graphiti.getPeService().getChopboxAnchor((AnchorContainer) pe);
        }
...
To get this code called, you need a shape with a ChopboxAnchor. I'll attach your test diagram where I deleted the second EClass shape, it's not needed here, and I added a ChopboxAnchor to the first EClass shape to get the connection creation button added to the button pad. I've done those modifications manually in the XML because the diagram doesn't run here, I hope it runs at your machine, else you need to do those modifications by yourself.

2) I forgot to clarify that the desired behavior is that the visual feedback for creating a connection should be positive when the target position is over the diagram itself, not over a target ContainerShape (we want our code to automatically create a target object when the mouse is released).
So, simplified, our TutorialCreateEReferenceFeature.canCreate(...) method looks like this:
  public void canCreate(...) {
    EClass source = getEClass(context.getSourceAnchor());
    EClass target = getEClass(context.getTargetPictogramElement());
    if (source != target) {
      return true;
    }
    return false;
  }
This should be enough to reproduce the problem: When dragging the mouse from the create connection context button to some diagram location, the visual feedback is negative but should be positive.

If you put a
System.out.println(target);
into the canCreate method, you see that when dragging over the diagram, target is not always null! Sometimes it is null, sometimes not. The problem seems to be the situation in GFDragConnectionTool as I described in my first posting.

Interesting fact is also that when starting the connection from the visual anchor on the ContainerShape, the visual feedback is correct.
Comment 7 Daniel Hoeh CLA 2017-01-31 09:31:00 EST
Created attachment 266542 [details]
Modified diagram with ChopboxAnchor and only one ContainerShape
Comment 8 Daniel Hoeh CLA 2017-01-31 09:36:27 EST
Created attachment 266543 [details]
Corrected diagram
Comment 9 Daniel Hoeh CLA 2017-01-31 09:37:15 EST
I just attached a corrected version of the test diagram, now it should work :-)
Comment 10 Michael Wenz CLA 2017-04-28 10:01:18 EDT
Daniel,

thanks for the detailed input on this. I was finally able to reproduce your issue and validate that your proposal wil solve this.

However, your change disables the option that is e.g. also used in the tutorial, to drag from the context button "Create Connection" from one EClass to another one to create a reference. So, I'm afraid this will need more investigations.

Michael