Find a way to get rid of inline code [#1940410]

Hello,

We should find a way to get rid of the inline code and use behaviors.
We must be sure that the Proj4JS behavior is runned before OpenLayers.

If we cannot be sure of that, we can maybe provide 2 functions

proj4js_load_definition() who's using behavior.
proj4js_load_definition_inline() who's using inline JS.

Comments

Comment #1

augustus.kling CreditAttribution: augustus.kling commented 12 March 2013 at 09:03

The inline code was brought in by my changes in ca00b422693a98e840d080bdbce7430c75700982. Let me explain why:

The existing code used drupal_add_js($settings, 'setting'); to provide the projection definition as Drupal settings. The openlayers module does the same to send its maps to the browser. Within Drupal these calls get combined into a single JavaScript object (in drupal_get_js where you find "jQuery.extend(Drupal.settings"). Afterwards the settings get applied in Drupal.attachBehaviors in Drupal's JavaScript.

In general this process is fine but it does not guarantee any execution order. Browsers do retain the order of the elements in the JavaScript in most cases – not that this is purely implementation dependent, varies across browsers and might change with each browser version. For details refer to https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Statements... or the discussion in http://code.google.com/p/v8/issues/detail?id=164 or near the top of page 92 in http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-262.pdf.

Looking at an example site, the definition of EPSG:42304 gets added before a Canadian map. Given the browser does retain iteration order in JavaScript objects the code will work just fine. However neither JavaScript nor ECMA-262 do guarantee the order so the behaviour of the browser is more or less coincidental. Once a browser is in use that does not retain the order (which is perfectly valid), the definition of EPSG:42304 is not there when OpenLayers initializes the map in which case coordinate transformations will yield wrong numbers.

I think we don't need a proj4js_load_definition_inline because it does not matter if the code runs inline. What matters is that the projection code runs before OpenLayers and after Proj4js was included. I've tried to guarantee this with the JS_LIBRARY group and the varying weights.

If the inline code should disappear, one needs to have another way of loading the projection definitions that is equally reliable. Obviously, this way should not cause a synchronous/blocking request for each projection definition.
For now I suggest to keep the inline code due to reliability and I have still to be convinced that a few lines of inline code hurt.

Comment #2

Pol

the/that

Brussels

CreditAttribution: Pol commented 12 March 2013 at 09:33

You're perfectly right.

I also agree that a few lines of inline code doesn't hurt.

What we can also do is, in openlayers.js from OpenLayers module, before creating the map, detect if there are definitions in Drupal.settings.proj4js, and if yes, process them.

Sample code:

...
// Use try..catch for error handling.
try {
  // Set OpenLayers language based on document 
  // rather than browser language
  OpenLayers.Lang.setCode($('html').attr('lang'));
  
  var Proj4JS = Proj4JS || {};
  for (var i in Drupal.settings.proj4js) {
    Proj4js.defs[settings.proj4js[i][0]] = settings.proj4js[i][1];
  }

...

Shouldn't it be better ?

Comment #3

augustus.kling CreditAttribution: augustus.kling commented 13 March 2013 at 14:33

Status:

Active

» Fixed

I changed the code now so that it uses a modified variant of your proposal. Thus we use Drupal's setting but guarantee the initialization order ourselves.

Comment #4

Pol

the/that

Brussels

CreditAttribution: Pol commented 13 March 2013 at 15:22

This is perfect :-) Thanks !

Comment #5

Pol

the/that

Brussels

CreditAttribution: Pol commented 13 March 2013 at 15:23

Do you think that proj4js module should be a dependency of OpenLayers now ?

Comment #6

augustus.kling CreditAttribution: augustus.kling commented 13 March 2013 at 15:28

Status:

Fixed

» Closed (fixed)

Do you think that proj4js module should be a dependency of OpenLayers now ?

Definitely, we need it in some cases and having Proj4js in the others does not hurt. We should try to reduce conditionally executed code as much as possible as this makes maintenance unnecessarily complex.
Beside that, it is only obvious to a user that the presence of proj4js changes the behaviour of openlayers when it is listed as dependency.