Bug 225353 - Null anchors not supported by the ShapeNodeEditPart
Summary: Null anchors not supported by the ShapeNodeEditPart
Status: RESOLVED FIXED
Alias: None
Product: GMF-Runtime
Classification: Modeling
Component: General (show other bugs)
Version: 2.0   Edit
Hardware: PC Windows XP
: P3 major
Target Milestone: 2.1.2   Edit
Assignee: Anthony Hunter CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2008-04-02 11:02 EDT by Romain Raugi CLA
Modified: 2011-05-17 17:41 EDT (History)
3 users (show)

See Also:


Attachments
Null anchor support for the ShapeNodeEditPart (1.39 KB, patch)
2008-05-19 08:35 EDT, Romain Raugi CLA
no flags Details | Diff
Null anchor support for the ShapeNodeEditPart (1.39 KB, patch)
2008-05-19 08:39 EDT, Romain Raugi CLA
no flags Details | Diff
Null anchor support for the ShapeNodeEditPart (1.44 KB, patch)
2008-09-08 05:19 EDT, Romain Raugi CLA
ahunter.eclipse: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Romain Raugi CLA 2008-04-02 11:02:22 EDT
NodeFigure.getSourceConnectionAnchorAt(Point) and NodeFigure.getTargetConnectionAnchor(Point) accept a null value and return the center of the figure if this is the case (see NodeFigure.createConnectionAnchor(Point)).

But ShapeNodeEditPart, that invokes them via their getSourceConnectionAnchor(Request) and getTargetConnectionAnchor(Request) method implementations, throws an NPE if no anchors are specified. This is because these methods try to make a copy of the anchor location:

ShapeNodeEditPart line 143 and 191:
Point pt = ((DropRequest) request).getLocation().getCopy();

I think this copy is useless since NodeFigure.createConnectionAnchor(Point) already creates a copy of the point given as an argument.

Furthermore, if you still prefer to work with a copy, please support the null location use case.
Comment 1 Romain Raugi CLA 2008-05-19 08:35:29 EDT
Created attachment 100895 [details]
Null anchor support for the ShapeNodeEditPart
Comment 2 Romain Raugi CLA 2008-05-19 08:39:52 EDT
Created attachment 100897 [details]
Null anchor support for the ShapeNodeEditPart

This patch removes the copy done on the location point, what makes the null case ok. This copy is useless since getNodeFigure().getSourceConnectionAnchorAt(pt) and getNodeFigure().getTargetConnectionAnchorAt(pt) already (indirectly) make a copy if the given point is not null.
Comment 3 Romain Raugi CLA 2008-05-19 08:50:35 EDT
It is critical for us that this (minor) fix is included in Ganymede release. Can you include it?

Thanks,
Comment 4 Romain Raugi CLA 2008-08-18 09:28:06 EDT
A workaround consists on overriding the methods getSourceConnectionAnchor(Request request) 
and getTargetConnectionAnchor(Request request) in your ShapeNodeEditPart implementations to support this case.

@Override
public ConnectionAnchor getSourceConnectionAnchor(Request request) {
  // Workaround for bug 225353
  if (request instanceof ReconnectRequest) {
    if (((DropRequest) request).getLocation() == null) {
      return getNodeFigure().getSourceConnectionAnchorAt(null);
    }
  }
  return super.getSourceConnectionAnchor(request);
}

@Override
public ConnectionAnchor getTargetConnectionAnchor(Request request) {
  // Workaround for bug 225353
  if (request instanceof ReconnectRequest) {
    if (((DropRequest) request).getLocation() == null) {
      return getNodeFigure().getTargetConnectionAnchorAt(null);
    }
  }
  return super.getTargetConnectionAnchor(request);
}
Comment 5 Anthony Hunter CLA 2008-09-04 10:14:33 EDT
(In reply to comment #0)
> 
> Furthermore, if you still prefer to work with a copy, please support the null
> location use case.
> 

Just to be completely paranoid, I would rather leave the getCopy() and just add the additional null check.

Can you create a patch for this and we will get into GMF 2.1.2?
Comment 6 Romain Raugi CLA 2008-09-08 05:19:51 EDT
Created attachment 111932 [details]
Null anchor support for the ShapeNodeEditPart
Comment 7 Anthony Hunter CLA 2008-09-16 19:41:09 EDT
Committed to HEAD and R2_1_maintenance
Comment 8 Anthony Hunter CLA 2008-09-16 21:51:43 EDT
Committed to HEAD and R2_1_maintenance
Comment 9 Eclipse Webmaster CLA 2010-07-19 12:30:27 EDT
[GMF Restructure] Bug 319140 : product GMF and component Runtime Diagram was the original product and component for this bug