getComponentString causes getShortString to fail if a geometry has only two components

[openlayers]

getShortString() fail if point parameter isn't define. It's can appends when call from getComponentsString()

getShortString must be change into:

getShortString: function(point) {

if( point ){

var resolution = this.getResolution();
var x = (point.x / resolution + this.left);
var y = (this.top - point.y / resolution);

if (this.inValidRange(x, y)) {

return x + "," + y;

} else {

return false;

}

}
return false;

},

[ahocevar]

I'm not sure if this is the right place to fix it. If there is a null component (the only reason I can think of where getShortString would be called with an empty point from getComponentString), then it would have to be fixed where such components are created.

[euzuro]

my sources tell me this is not a regression, so I'm moving it to 2.8 milestone.

[openlayers]

You're right ahocevar. The problem appends in getComponentString() if the parameter components contains only 2 points.

646 	    getComponentsString: function(components, separator) {
647 	        var renderCmp = [];
648 	        var complete = true;
649 	        var len = components.length;
650 	        var strings = [];
651 	        var str, component, j;
652 	        for(var i=0; i<len; i++) {
653 	            component = components[i];
654 	            renderCmp.push(component);
655 	            str = this.getShortString(component);
656 	            if (str) {
657 	                strings.push(str);
658 	            } else {
659 	                // The current component is outside the valid range. Let's
660 	                // see if the previous or next component is inside the range.
661 	                // If so, add the coordinate of the intersection with the
662 	                // valid range bounds.
663 	                if (i > 0 && i < len - 1) { //<-------------------- just add "&& i < len - 1" here
664 	                    if (this.getShortString(components[i + 1])) {
665 	                        strings.push(this.clipLine(components[i],
666 	                            components[i-1]));
667 	                    }
668 	                }
669 	                if (i < len - 1) {
670 	                    if (this.getShortString(components[i + 1])) {
671 	                        strings.push(this.clipLine(components[i],
672 	                            components[i+1]));
673 	                    }
674 	                }
675 	                complete = false;
676 	            }
677 	        }
678 	
679 	        return {
680 	            path: strings.join(separator || ","),
681 	            complete: complete
682 	        };
683 	    },

[openlayers]

Sorry to insist but the problem still exists in the 2.7RC2 (http://openlayers.org/api/2.7-rc2/OpenLayers.js). It's really simple to fix it. Why wait for the 2.8 milestone ?

[ahocevar]

Ok, thanks anonymous for tracking this down. This is indeed a regression from 2.6, introduced by r7930. Patch to come.

[ahocevar]

getComponentString causes getShortString to fail if a geometry has only two components

[ahocevar]

Actually, the problem here was just a fatal typo. The above patch fixes it, and adds tests that prove that the issue is definitely caused by this typo. Thanks anonymous for catching this.

Relevant tests pass in FF2 and FF3. Thanks for any review.

[pagameba]

Looks good to me, I like the new tests. I think this needs to be changed to pullup after commit to make it into the branch.

[ahocevar]

(In [8016]) Fixed fatal typo that broke getComponentString when working with two-point geometries. Added tests to check that everything works as expected now. r=pagameba (pullup #1730)

[euzuro]

(In [8038]) Bringing up patches from trunk to branch for RC3. -r8012:HEAD. (Closes #1594) (Closes #1730) (Closes #1735) (Closes #1738) (Closes #1740)