doc/bugs/osm_plugin_error_TypeError:_mapProjection_is_null.mdwn

   1 [[!template  id=gitbranch branch=cbaines/osm-layers-patch author="[[cbaines]]"]]
   2
   3 Using the osm plugin with a simple \[[!osm]] directive does not seem to work, a "TypeError: mapProjection is null" is given. I believe this is because the client side Javascript uses the options.layers, which is always Null.
   4
   5 [[!tag patch]]
   6 I have produced a patch for this issue, but beware, while it appears to fix the problem for me, I have little understanding of perl and the existing code base.
   7
   8 > It looks sound, but I have yet to test it. --[[anarcat]]
   9
  10 >> I reviewed a version of this (possibly rebased or modified or something)
  11 >> that was in the [[todo/osm_plugin_GeoJSON_popup_patch]] branch,
  12 >> over on the todo page for that branch. Feel free to move my
  13 >> review comments for it here if you want to split the discussion. --[[smcv]]
  14 >> [[!tag reviewed]]
  15
  16 Here's [[smcv]]'s review from [[todo/osm_plugin_GeoJSON_popup_patch]], annotated with my comments. --[[anarcat]]
  17
  18 > It would be good if the commit added documentation for the new feature,
  19 > probably in `doc/ikiwiki/directive/osm.mdwn`.
  20 >
  21 >     + my @layers = [ 'OSM' ];
  22 >
  23 > You mean `$layers`. `[]` is a scalar value (a reference to an array);
  24 > `@something` is an array.
  25
  26 >> Or `@layers = ( 'OSM' );`. --[[anarcat]]
  27
  28 >     +         @layers = [ split(/,/, $params{layers}) ];
  29 >
  30 > Is comma-separated the best fit here? Would whitespace, or whitespace and/or
  31 > commas, work better?
  32
  33 >> Why don't we simply keep it an array as it already is? I fail to see the reason behind that change. This is the config I use right now on http://reseaulibre.ca/:
  34 >>
  35 >> ~~~~
  36 >> osm_layers:
  37 >> - http://a.tile.stamen.com/toner/${z}/${x}/${y}.png
  38 >> - OSM
  39 >> - GoogleHybrid
  40 >> ~~~~
  41 >>
  42 >> It works fine. At the very least, we should *default* to the configuration set in the the .setup file, so this chunk of the patch should go:
  43 >>
  44 >> ~~~~
  45 >> -        $options{'layers'} = $config{osm_layers};
  46 >> ~~~~
  47 >>
  48 >> Maybe the best would be to use `$config{osm_layers};` as a default? --[[anarcat]]
  49
  50 > It's difficult to compare without knowing what the values would look like.
  51 > What would be valid values? The documentation for `$config{osm_layers}`
  52 > says "in a syntax acceptable for OpenLayers.Layer.OSM.url parameter" so
  53 > perhaps:
  54 >
  55 >     # expected by current branch
  56 >     \[[!osm layers="OSM,WTF,OMG"]]
  57 >     \[[!osm layers="http://example.com/${z}/${x}/${y}.png,http://example.org/tiles/${z}/${x}/${y}.png"]]
  58 >     # current branch would misbehave with this syntax but it could be
  59 >     made to work
  60 >     \[[!osm layers="OSM, WTF, OMG"]]
  61 >     \[[!osm layers="""http://example.com/${z}/${x}/${y}.png,
  62 >       http://example.org/tiles/${z}/${x}/${y}.png"""]]
  63 >     # I would personally suggest whitespace as separator (split(' ', ...))
  64 >     \[[!osm layers="OSM WTF OMG"]]
  65 >     \[[!osm layers="""http://example.com/${z}/${x}/${y}.png
  66 >       http://example.org/tiles/${z}/${x}/${y}.png"""]]
  67 >
  68 > If you specify more than one layer, is it like "get tiles from OpenCycleMap
  69 > server A or B or C as a round-robin", or "draw OpenCycleMap and then overlay
  70 > county boundaries and then overlay locations of good pubs", or what?
  71
  72 >> Multiple layers support means that the user is shown the first layer by default, but can also choose to flip to another layer. See again http://reseaulibre.ca/ for an example. --[[anarcat]]
  73
  74 >     +         layers => @layers,
  75 >
  76 > If @layers didn't have exactly one item, this would mess up argument-parsing;
  77 > but it has exactly one item (a reference to an array), so it works.
  78 > Again, if you replace @layers with $layers throughout, that would be better.
  79 >
  80 >     -        $options{'layers'} = $config{osm_layers};
  81 >
  82 > Shouldn't the default if no `$params{layers}` are given be this, rather
  83 > than a hard-coded `['OSM']`?
  84
  85 >> Agreed. --[[anarcat]]
  86
  87 > `getsetup()` says `osm_layers` is `safe => 0`, which approximately means
  88 > "don't put this in the web UI, changing it could lead to a security flaw
  89 > or an unusable website". Is that wrong? If it is indeed unsafe, then
  90 > I would expect changing the same thing via \[[!osm]] parameters to be
  91 > unsafe too.
  92
  93 >> I put that at `safe=>0` as a security precaution, because I didn't
  94 >> exactly know what that setting did.
  95 >>
  96 >> It is unclear to me whether this could lead to a security flaw. The
  97 >> osm_layers parameter, in particular, simply decides which tiles get
  98 >> loaded in OpenLayers, but it is unclear to me if this is safe to change
  99 >> or not. --[[anarcat]]
 100
 101 > I notice that `example => { 'OSM', 'GoogleSatellite' }` is wrong:
 102 > it should (probably) be `example => [ 'OSM', 'GoogleSatellite' ]`
 103 > (a list of two example values, not a map with key 'OSM' corresponding
 104 > to value 'GoogleSatellite'. That might be why you're having trouble
 105 > with this.
 106
 107 >> That is an accurate statement.
 108 >>
 109 >> This is old code, so my memory may be cold, but i think that the "layers" parameters used to be a hash, not an array, until two years ago (commit 636e04a). The javascript code certainly expects an array right now. --[[anarcat]]