Only valid characters in generated ids.

lib/OpenLayers/Control/LayerSwitcher.js

@@ -7,6 +7,7 @@
  * @requires OpenLayers/Control.js
  * @requires OpenLayers/Lang.js
  * @requires OpenLayers/Console.js
+ * @requires OpenLayers/Util.js
  * @requires OpenLayers/Events/buttonclick.js
  */
 
@@ -52,7 +53,6 @@ OpenLayers.Control.LayerSwitcher =
      */
     layerStates: null,
 
-
   // DOM Elements
 
     /**
@@ -325,8 +325,14 @@ OpenLayers.Control.LayerSwitcher =
                                           : layer.getVisibility();
 
                 // create input element
-                var inputElem = document.createElement("input");
-                inputElem.id = this.id + "_input_" + layer.name;
+                var inputElem = document.createElement("input"),
+                    // The input shall have an id attribute so we can use
+                    // labels to interact with them.
+                    inputId = OpenLayers.Util.createUniqueID(
+                        this.id + "_input_"
+                    );
+
+                inputElem.id = inputId;
                 inputElem.name = (baseLayer) ? this.id + "_baseLayers" : layer.name;
                 inputElem.type = (baseLayer) ? "radio" : "checkbox";
                 inputElem.value = layer.name;
@@ -342,6 +348,8 @@ OpenLayers.Control.LayerSwitcher =
 
                 // create span
                 var labelSpan = document.createElement("label");
+                // this isn't the DOM attribute 'for', but an arbitrary name we
+                // use to find the appropriate input element in <onButtonClick>
                 labelSpan["for"] = inputElem.id;
                 OpenLayers.Element.addClass(labelSpan, "labelSpan olButton");
                 labelSpan._layer = layer.id;

tests/Control/LayerSwitcher.html

@@ -88,13 +88,13 @@
         control = new OpenLayers.Control.LayerSwitcher();
         map.addControl(control);
 
-        var wmsInput = OpenLayers.Util.getElement(control.id + "_input_" + layer.name);
+        var wmsInput = control.div.childNodes[0].childNodes[1].childNodes[0];
         t.ok(wmsInput != null, "correctly makes an input for wms layer");
         t.eq(wmsInput.type, "radio", "wms correctly made a radio button");
         t.eq(wmsInput.name, control.id + "_baseLayers", "wms correctly named");
         t.eq(wmsInput.value, layer.name, "wms correctly valued");
         
-        var markersInput = OpenLayers.Util.getElement(control.id + "_input_" + markers.name);
+        var markersInput = control.div.childNodes[0].childNodes[3].childNodes[0];
         t.ok(markersInput != null, "correctly makes an input for markers layer");
         t.eq(markersInput.type, "checkbox", "wms correctly made a radio button");
         t.eq(markersInput.name, markers.name, "wms correctly named");
@@ -191,7 +191,54 @@
         t.eq(control.div.childNodes[0].childNodes[0].style.display, "" , "Base layer display on when visble base layer");
     }
 
+    // See e.g. https://github.com/openlayers/openlayers/issues/866
+    function test_Control_LayerSwitcher_validIds(t){
+        t.plan(2);
 
+        // setup
+        var layername = "Name with spaces & illegal characters * + ~ ` ' ? )",
+            map = new OpenLayers.Map("map", {
+                controls: [
+                    new OpenLayers.Control.LayerSwitcher()
+                ],
+                layers: [
+                    new OpenLayers.Layer.WMS(
+                        layername,
+                        "http://example.com/"
+                    ),
+                    // add another layer with the same name, the generated id
+                    // must be different
+                    new OpenLayers.Layer.WMS(
+                        layername,
+                        "http://example.com/"
+                    )
+                ]
+            });
+
+        var baselayerDiv = map.controls[0].div.childNodes[0].childNodes[1],
+            firstGeneratedInputId = baselayerDiv.childNodes[0].id,
+            secondGeneratedInputId = baselayerDiv.childNodes[1].id,
+            // legal ids start with a letter and are followed only by word
+            // characters (letters, digits, and underscores) plus the dash (-)
+            // This is only a subset of all allowed charcters inside of ids.
+            allowedIdChars = (/^[a-zA-Z]{1}[\w-]*$/g);
+
+        // tests
+        // validity
+        t.ok(
+            allowedIdChars.test(firstGeneratedInputId),
+            "id only contains letters, digits, underscores and dashes. It " +
+                "starts with a letter."
+        );
+        // uniqueness
+        t.ok(
+            firstGeneratedInputId !== secondGeneratedInputId,
+            "generated ids are different even for equal layernames"
+        );
+
+        // teardown
+        map.destroy();
+    }
 
   </script>
 </head>