OpenLayers OpenLayers

Ticket #1274 (closed bug: fixed)

Opened 11 months ago

Last modified 11 months ago

SVG.drawCircle tries to remove non-added nodes

Reported by: crschmidt Assigned to:
Priority: minor Milestone: 2.6 Release
Component: Renderer.SVG Version: 2.5
Keywords: Cc:
State: Complete

Description (Last modified by crschmidt)

In a case where a node is out of range when it is added, the SVG renderer will complain about trying to add the node. You can likely reproduce this by trying to add a node that's way out of the current view. 

Index: ../../lib/OpenLayers/Renderer/SVG.js
===================================================================
--- ../../lib/OpenLayers/Renderer/SVG.js        (revision 5743)
+++ ../../lib/OpenLayers/Renderer/SVG.js        (working copy)
@@ -330,15 +330,16 @@
             node.setAttributeNS(null, "cy", y);
             node.setAttributeNS(null, "r", radius);
         } else {
-            this.root.removeChild(node);
+            if (node.parentNode) {
+                this.root.removeChild(node);
+            }    
         }    
     },

Attachments

renderer.patch (1.7 kB) - added by crschmidt on 01/20/08 10:12:29.
with tests
svg.patch (1.8 kB) - added by crschmidt on 01/20/08 15:07:19.

Change History

01/15/08 11:53:21 changed by crschmidt

(In [5749]) Fix to SVG renderer. (See #1274)

01/20/08 10:12:29 changed by crschmidt

  • attachment renderer.patch added.

with tests

01/20/08 10:14:15 changed by crschmidt

  • description changed.

Patch with tests attached. The problem is that if you attempt to draw data that is far enough out of your view, it throws an error because it tries to remove the node (instead of adding/modifying it), but it hasn't yet been added, so it fails.

In general, we only request data within a bounding box that we can see in OpenLayers code, which is why we seldom run into this bug.

01/20/08 10:14:23 changed by crschmidt

  • state set to Review.

01/20/08 13:02:52 changed by pspencer

  • state changed from Review to Needs Discussion.

Is it sufficient to test node.parentNode or should it test if node.parentNode == this.root? Or change the next line to node.parentNode.removeChild(node).

I'm not sure how this code gets used and if it is possible to call with a node that has a parentNode that isn't this.root, in which case a note to that effect in the code would be useful.

01/20/08 15:05:36 changed by crschmidt

  • state changed from Needs Discussion to Needs More Work.

Using the API, the only way that a circle node exists would be to add it to the root -- nodes are not added to anything other than the root. I figured the check for "does it have a parent node" would possibly be cheaper than the check for "does it have a parent node, and is the parent node equal to $this", but have no real reason to support that. It's probably safer to just ensure that it's all the way correct: I'll upload a new patch.

01/20/08 15:07:19 changed by crschmidt

  • attachment svg.patch added.

01/20/08 15:07:43 changed by crschmidt

  • state changed from Needs More Work to Review.

New patch attached. Tests still seem to pass.

01/20/08 15:10:13 changed by pspencer

  • state changed from Review to Commit.

looks good.

01/20/08 15:11:38 changed by crschmidt

  • status changed from new to closed.
  • state changed from Commit to Complete.
  • resolution set to fixed.

(In [5826]) Don't let the SVG renderer bail if the node we are trying to add is out of bounds *and* not yet added to the map. r=pagameba (Closes #1274)