OpenLayers OpenLayers

Ticket #666 (closed bug: fixed)

Opened 2 years ago

Last modified 1 year ago

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

Reported by: openlayers Assigned to:
Priority: critical Milestone: 2.5 Release
Component: Feature.Vector Version: SVN
Keywords: Cc:
State:

Description (Last modified by crschmidt)

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

Attachments

handler_bounds.patch (7.6 kB) - added by crschmidt on 08/23/07 23:18:33.
handler_bounds.2.patch (7.7 kB) - added by tschaub on 08/24/07 18:28:20.
slightly modified patch
handler_bounds_polygon.patch (1.6 kB) - added by pgiraud on 09/11/07 10:04:37.
added missing clearBounds() call in Handler.Polygon and modified test

Change History

04/13/07 19:03:04 changed by crschmidt

  • milestone changed from 2.4 Release to 2.5 Release.

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.

07/15/07 03:08:43 changed by crschmidt

  • summary changed from Vector drawing in Internet Explorer to EditingToolbar does not display first segment in VML until 3 points added.

07/15/07 03:09:12 changed by crschmidt

  • description changed.

07/19/07 07:48:15 changed by 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.

07/19/07 09:32:39 changed by 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();

...

07/19/07 09:37:47 changed by 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.

07/19/07 10:46:21 changed by 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.

08/03/07 11:42:36 changed by crschmidt

  • type changed from feature to bug.

08/23/07 23:16:14 changed by crschmidt

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

08/23/07 23:18:33 changed by crschmidt

  • attachment handler_bounds.patch added.

08/23/07 23:19:36 changed by crschmidt

  • keywords set to needstests.

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.

08/24/07 17:27:22 changed by crschmidt

  • keywords changed from needstests to review.

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.

08/24/07 18:28:20 changed by tschaub

  • attachment handler_bounds.2.patch added.

slightly modified patch

08/24/07 18:29:18 changed by tschaub

  • keywords changed from review to commit.

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).

08/24/07 18:33:30 changed by crschmidt

  • keywords deleted.
  • status changed from new to closed.
  • resolution set to fixed.

(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.

08/24/07 23:40:03 changed by tschaub

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

09/11/07 09:28:27 changed by pgiraud

  • status changed from closed to reopened.
  • resolution deleted.

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.

09/11/07 10:04:37 changed by pgiraud

  • attachment handler_bounds_polygon.patch added.

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

09/11/07 10:05:18 changed by pgiraud

  • keywords set to review.

09/11/07 10:16:38 changed by pgiraud

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

09/11/07 10:22:11 changed by crschmidt

  • keywords changed from review to commit.

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!

09/11/07 10:26:09 changed by crschmidt

  • keywords deleted.
  • status changed from reopened to closed.
  • resolution set to fixed.

(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)