Check for existing prototypes on objects and don't clobber them

[crschmidt]

See #961: OL is clobbering Ajax.net's String.prototype.trim. Note that in this case, fixing our trim should be fine, but in general, we should be more careful about our overrides.

[crschmidt]

(In [4213]) Pull in upstream fix from Prototype, patch by fredj (thanks fredj!) to fix String.prototype.trim. (Closes #961) Note that we should also be more careful not to clobber other library prototypes. (See #962) I'm going to check this in to fix the bug for 2.5, and we'll work on the latter in 2.6.

[fredj]

BTW, why do we have a String.indexOf in BaseTypes.js ? Standard ECMA-262 already define this function

Is the function not defined in some browser ?

[fredj]

Replying to fredj:

BTW, why do we have a String.indexOf in BaseTypes.js ? Standard ECMA-262 already define this function Is the function not defined in some browser ?

see #963

[euzuro]

out of curiosity, are we implying an approach something like

if (!String.prototype.trim) {

String.prototype.trim = function() {

... ...

}

}

?

If so, is that safe? In the case of trim(), the functionality seems to be pretty standard/universal. But in the case of something like Function.prototype.bind(), that might not be the case.... no?

[crschmidt]

Yep, that's the implication.

[euzuro]

after potatoe chips cr5 and i decided that probably the best appropriate way to deal with this is to do what i've mentioned above except to raise a Console.error() if the function *is* defined.

Those tackon prototype functions are necessary for OL to work correctly and if there is a collision it needs to be reported as an error.

I'll take this on but not until 2.6

[tschaub]

#712 pleeease.

[tschaub]

In case I wasn't descriptive enough in my comment above, I really think we should take an alternative tack. Instead of adding sugar to the language, lets act like a library.

Unfortunately, nobody else writes small libraries that we can import, so we're stuck reimplementing extra stuff we want. As long as we are relying on functionality that the language doesn't give us, it belongs in our library - and in our namespace. We don't want to make decisions based on any other names (like String.prototype.trim or Function.prototype.bind) - we only know about names that start with OpenLayers - because we clobbered whatever else someone else may have defined there.

So, really, please consider moving away from these sugary sweet language extensions. And I don't believe it all has to wait until 3.0. I'd rather see OpenLayers.Function.bind in 2.6 instead of if(Function.prototype.bind) {OpenLayers.Console.error("yikes, I wanted to do that")}.

[crschmidt]

So, with #712 going in, we've made the situation inside of OpenLayers better -- we no longer depend on function.bind being what we want.

However, we want to provide backwards compatibility -- and the new way of providing backwards compatibility seems to break Safari and Opera.

It seems that the extensions we do to bind are not compatible with Prototype -- at least version 1.6.0 -- so the mixing and matching of prototype with OpenLayers causes problems.

Patch incoming.

[crschmidt]

This fixes the current problem originally described in #977. (There are other things to do there, but this is a start.)

[euzuro]

all good. ff & ie6 are happy. please commit

[crschmidt]

(In [4339]) Don't clobber existing prototypes. Since OpenLayers doesn't use 'foo.bind' anymore, preferring OpenLayers.Function.bind(function, object), we don't need to provide this -- so if someone provides something different, don't clobber it. (Closes #962)

[crschmidt]

(In [6035]) Minor modifications to the map.setCenter / map.moveTo rewrite. (See #962)

[pgiraud]

Replying to crschmidt:

(In [6035]) Minor modifications to the map.setCenter / map.moveTo rewrite. (See #962)

Correct me if I'm wrong but this should have been : (In [6035]) Minor modifications to the map.setCenter / map.moveTo rewrite. (See #967)

[crschmidt]

RIght you are!