Bug 328678

Description Mathieu Velten 2010-10-26 04:50:05 EDT

The biggest issue I found is that the current common class bypasses the gmf command framework, which leads to unwanted behavior because the policies are ignored.

For example in getDefaultDropNodeCommand we are building a createcommand and a setbounds command instead of asking the edit part to give us a fully complete command using a request.

An other problem is that we change the location of the request to a relative one before calling the abstract method.
I agree it could be convenient to already have relative location in the sub classes but it is not correct regarding the framework which uses absolute location in its requests.
A protected helper method to translate from absolute coordinates to relative should be sufficient.

Since this problem affects the behavior of an overridden method, most implementations need to be corrected regarding this point.

You can find attached an incomplete patch.
It fixes the common class according to this remarks and the sequence subclass for giving an example.

here is the procedure to fix each one of them :

- take care not to bypass the command framework. replace all new CreateCommand() by a call to getDefaultDropNodeCommand or dropBinaryLink, or at least use hostEditPart.getCommand if you need more control. And there is quite a lot of them :).
- if you do really use location in your algo (ie not just passing it to getDefaultDropNodeCommand or dropBinaryLink) you should translate it to relative before so that your algo behaves the same way as before. You can use getTranslatedToRelative method for that. And don't forget to use the original absolute coordinates for calling getDefaultDropNodeCommand and dropBinaryLink, or for using a request.
- in your methods (and method signatures) try to use absoluteLocation or relativeLocation variable names to keep track of which one is in use.