EditingToolbar does not display first segment in VML until 3 points added

[openlayers]

When trying to add a simple line (from point a to point b) in IE, the line does not appear on the map. It does only work to draw a line with more than 2 points in it (for example from point A over point B to point C. I'm talking about the line function on this example (latest release I guess): http://openlayers.org/dev/examples/wkt.html

[crschmidt]

This is a deficiency in the VML renderer. Without a patch, I'm going to push this off to 2.5. It sucks, but the VML layer is being told the right thing to do, it's just not doing it, which makes it a hard problem to solve.

[pgiraud]

I made some investigations about that. I think I found what's wrong.

In the VML renderer, the drawLine method calls the setNodeDimension method to set the coordOrigin and coordSize attributes for the VML line node. It relies on geometry.getBounds(). Moving the mouse using the path handler modifies the handler temporary line (drawFeature) though in a way that doesn't update the geometry bounds. Then, until the second line component is added, the vml node has a coordSize set to (0,0).

Will try to propose a patch soon.

[pgiraud]

Resetting bounds to null either for the geometry and its components in setNodeDimension before calling getBounds (VML.js) does the job. Though, it doesn't feel great to me, it's just more like a workaround.

setNodeDimension: function(node, geometry) {

for (var i = 0; i < geometry.components.length; i++) {

geometry.components[i].bounds = null;

} geometry.bounds = null; var bbox = geometry.getBounds();

...

[crschmidt]

It seems to me like the correct solution for this is to call this.line.geometry.clearBounds() in the Path.js modifyFeature. clearBounds is definitely the right function to use, and I think this is the right place to use it.

Tim, please take a look at this when you get a chance.

I have a feeling that the reason this hasn't been done up until now is that it may be a performance hit. if that's the case, then we should find out how much of one, and figure out a work around.

[pgiraud]

Using clearBounds, we can clear the last component bounds and the parent geometry as well :

setNodeDimension: function(node, geometry) {
        // clear the geometry bounds
        geometry.components[geometry.components.length - 1].clearBounds();

        var bbox = geometry.getBounds();

This works, but is not good for performances.

[crschmidt]

I think that we should just go ahead and fix this, and then making performance improvements can come as a second pass.

[crschmidt]

I'm not 100% sure on this one, but it seems like this is the right direction: need to test if this actually fixes the bug. (I tried to add code in the Polygon to do something similar, but couldn't create a test case for it, suggesting I may be doing something wrong.)

Don't have IE here to test, so will look at it in the morning.

[crschmidt]

Pierre confirmed that this fixes the problem, which means that the lower level handlers are correctly dealing with the not-properly set bounds. Marking this one for review now.

[tschaub]

slightly modified patch

[tschaub]

Nice work on the tests Chris! I only added plans and removed a console call. All pass in IE and FF. Please commit (the second patch).

[crschmidt]

(In [4036]) EditingToolbar does not display first segment in VML until 3 points added. This fixes the bounds, which fixes the vml behavior in IE. (Closes #666.) Hooray, no more devil ticket.

[tschaub]

(In [4039]) Adding two test pages that didn't make it in r4036 - that devilish ticket. (See #666.)

[pgiraud]

I'm sorry but the given patch doesn't do the job for polygons. One more test is needed. I will add a new patch for that and the assciated test.

[pgiraud]

added missing clearBounds() call in Handler.Polygon and modified test

[pgiraud]

(In [4215]) added clearBounds call, fixes problem on IE with the first line draw (see #666)

[crschmidt]

Pierre: Thanks. I had tried to test this -- as you can tell - but it hadn't occured to me that my test was backwards, which is why I left his specific bit of the changes I thought were needed out.

I'll take care of this one. Thanks again!

[crschmidt]

(In [4217]) Fix lacking code from #666-- I had originally wanted to add this, but couldn't craft a test which exposed the lack ... until i realized with pgiraud's help that my test was backwards to begin with. (Closes #666)