Handler.RegularPolygon: Strange behavior

[jorix]

Using http://openlayers.org/dev/examples/regular-polygons.html select "draw polygon" and:

1 Try:

• keep left mousedown.
• move
• right mousedown.
• release the left mouse button and then the right.
• Oops!: The blue polygon remains.
• move and click. Oops! The blue polygon is converted to a final polygon but centered on the click.

2 Try:

• keep left mousedown.
• move cursor out the map
• Oops! the polygon is definitive.

[jorix]

attachment:3199.patch solves "Try 2". This patch together with patch #3193 also solves the "Try 1".

Please review.

[erilem]

Instead of finalizing or canceling the drawing I think we should allow the user to continue drawing when the mouse re-enters the map, and this is the behavior we have with the Point, Path and Polygons handlers. But I agree that canceling leads to a better behavior than finalizing.

I'm confirming that the RegularPolygon tests pass in IE8, and FF4.

[erilem]

(In [11770]) when drawing a regular polygon cancel drawing instead of finalizing when the mouse leaves the map, p=jorix, r=me,sbrunner (closes #3199)

[fredj]

"Try 2" case: when the mouse out of the map, I have the following error:

Uncaught TypeError: Cannot call method 'clone' of null

[fredj]

proposed patch:

Index: lib/OpenLayers/Handler/RegularPolygon.js
===================================================================
--- lib/OpenLayers/Handler/RegularPolygon.js	(revision 11785)
+++ lib/OpenLayers/Handler/RegularPolygon.js	(working copy)
@@ -407,8 +407,11 @@
     callback: function (name, args) {
         // override the callback method to always send the polygon geometry
         if (this.callbacks[name]) {
-            this.callbacks[name].apply(this.control,
-                                       [this.feature.geometry.clone()]);
+            var geom = null;
+            if (this.feature && this.feature.geometry) {
+                geom = this.feature.geometry.clone();
+            }
+            this.callbacks[name].apply(this.control, [geom]);
         }
         // since sketch features are added to the temporary layer
         // they must be cleared here if done or cancel

[erilem]

Ok, this patch actually creates new problems, so I'll revert it and set the milestone to 2.12.

[erilem]

Please set it back to 2.11 if you can come with a working patch.

[jorix]

NOTE: This ticket only analyzes the problem "2 Try" to a solution of "1 Try" see #3193

This issue is more complex than it seemed to me. For example when the handler is used by SLDSelect control this behavior is acceptable, even preferable (see examples/SLDSelect.html) So this is not strictly a bug.

This would lead to consider two behaviors when "mouseout", to call "done" or "cancel" callbacks. In the patch attachment/3199.2.patch I have incorporated these two behaviors and the proposed code by fredj changed to avoid a call to "done" callback after "cancel" (Handler.Drag can to call "done" callback on "mouseout").

The patch also includes tests and a adjustment in the example.

[jorix]

Merged with current trunk and changes in tests after [12346]

[jorix]

The introduction of the dummy layer in the  test_callbacks by [12346] forced to change the tests proposed.

Please review 3199.3.patch , thanks