From 39a5aed5a6b4d9cde070a6b5a05f407be294c45e Mon Sep 17 00:00:00 2001 From: Marc Jansen Date: Mon, 4 Feb 2013 15:45:19 +0100 Subject: [PATCH 1/3] Remove trailing whitespace. --- lib/OpenLayers/Control/LayerSwitcher.js | 226 ++++++++++++------------ 1 file changed, 113 insertions(+), 113 deletions(-) diff --git a/lib/OpenLayers/Control/LayerSwitcher.js b/lib/OpenLayers/Control/LayerSwitcher.js index 6f5ab78ed6..d4e686d8de 100644 --- a/lib/OpenLayers/Control/LayerSwitcher.js +++ b/lib/OpenLayers/Control/LayerSwitcher.js @@ -3,7 +3,7 @@ * See license.txt in the OpenLayers distribution or repository for the * full text of the license. */ -/** +/** * @requires OpenLayers/Control.js * @requires OpenLayers/Lang.js * @requires OpenLayers/Console.js @@ -12,18 +12,18 @@ /** * Class: OpenLayers.Control.LayerSwitcher - * The LayerSwitcher control displays a table of contents for the map. This + * The LayerSwitcher control displays a table of contents for the map. This * allows the user interface to switch between BaseLasyers and to show or hide - * Overlays. By default the switcher is shown minimized on the right edge of + * Overlays. By default the switcher is shown minimized on the right edge of * the map, the user may expand it by clicking on the handle. * - * To create the LayerSwitcher outside of the map, pass the Id of a html div + * To create the LayerSwitcher outside of the map, pass the Id of a html div * as the first argument to the constructor. - * + * * Inherits from: * - */ -OpenLayers.Control.LayerSwitcher = +OpenLayers.Control.LayerSwitcher = OpenLayers.Class(OpenLayers.Control, { /** @@ -37,104 +37,104 @@ OpenLayers.Control.LayerSwitcher = */ roundedCorner: false, - /** + /** * APIProperty: roundedCornerColor * {String} The color of the rounded corners, only applies if roundedCorner * is true, defaults to "darkblue". */ roundedCornerColor: "darkblue", - - /** - * Property: layerStates - * {Array(Object)} Basically a copy of the "state" of the map's layers + + /** + * Property: layerStates + * {Array(Object)} Basically a copy of the "state" of the map's layers * the last time the control was drawn. We have this in order to avoid * unnecessarily redrawing the control. */ layerStates: null, - + // DOM Elements - + /** * Property: layersDiv - * {DOMElement} + * {DOMElement} */ layersDiv: null, - - /** + + /** * Property: baseLayersDiv * {DOMElement} */ baseLayersDiv: null, - /** + /** * Property: baseLayers * {Array(Object)} */ baseLayers: null, - - - /** + + + /** * Property: dataLbl - * {DOMElement} + * {DOMElement} */ dataLbl: null, - - /** + + /** * Property: dataLayersDiv - * {DOMElement} + * {DOMElement} */ dataLayersDiv: null, - /** + /** * Property: dataLayers - * {Array(Object)} + * {Array(Object)} */ dataLayers: null, - /** + /** * Property: minimizeDiv - * {DOMElement} + * {DOMElement} */ minimizeDiv: null, - /** + /** * Property: maximizeDiv - * {DOMElement} + * {DOMElement} */ maximizeDiv: null, - + /** * APIProperty: ascending - * {Boolean} + * {Boolean} */ ascending: true, - + /** * Constructor: OpenLayers.Control.LayerSwitcher - * + * * Parameters: * options - {Object} */ initialize: function(options) { OpenLayers.Control.prototype.initialize.apply(this, arguments); this.layerStates = []; - + if(this.roundedCorner) { OpenLayers.Console.warn('roundedCorner option is deprecated'); } }, /** - * APIMethod: destroy - */ + * APIMethod: destroy + */ destroy: function() { - - //clear out layers info and unregister their events + + //clear out layers info and unregister their events this.clearLayersArray("base"); this.clearLayersArray("data"); - + this.map.events.un({ buttonclick: this.onButtonClick, addlayer: this.redraw, @@ -144,15 +144,15 @@ OpenLayers.Control.LayerSwitcher = scope: this }); this.events.unregister("buttonclick", this, this.onButtonClick); - + OpenLayers.Control.prototype.destroy.apply(this, arguments); }, - /** + /** * Method: setMap * * Properties: - * map - {} + * map - {} */ setMap: function(map) { OpenLayers.Control.prototype.setMap.apply(this, arguments); @@ -176,9 +176,9 @@ OpenLayers.Control.LayerSwitcher = * Method: draw * * Returns: - * {DOMElement} A reference to the DIV DOMElement containing the + * {DOMElement} A reference to the DIV DOMElement containing the * switcher tabs. - */ + */ draw: function() { OpenLayers.Control.prototype.draw.apply(this); @@ -191,7 +191,7 @@ OpenLayers.Control.LayerSwitcher = } // populate div with current info - this.redraw(); + this.redraw(); return this.div; }, @@ -224,13 +224,13 @@ OpenLayers.Control.LayerSwitcher = } }, - /** + /** * Method: clearLayersArray * User specifies either "base" or "data". we then clear all the * corresponding listeners, the div, and reinitialize a new array. - * + * * Parameters: - * layersType - {String} + * layersType - {String} */ clearLayersArray: function(layersType) { this[layersType + "LayersDiv"].innerHTML = ""; @@ -241,9 +241,9 @@ OpenLayers.Control.LayerSwitcher = /** * Method: checkRedraw * Checks if the layer state has changed since the last redraw() call. - * + * * Returns: - * {Boolean} The layer state changed since the last redraw() call. + * {Boolean} The layer state changed since the last redraw() call. */ checkRedraw: function() { var redraw = false; @@ -254,41 +254,41 @@ OpenLayers.Control.LayerSwitcher = for (var i=0, len=this.layerStates.length; i Date: Mon, 4 Feb 2013 16:43:34 +0100 Subject: [PATCH 2/3] Only valid characters in generated ids. --- lib/OpenLayers/Control/LayerSwitcher.js | 14 +++++-- tests/Control/LayerSwitcher.html | 51 ++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/lib/OpenLayers/Control/LayerSwitcher.js b/lib/OpenLayers/Control/LayerSwitcher.js index d4e686d8de..3e4972d28b 100644 --- a/lib/OpenLayers/Control/LayerSwitcher.js +++ b/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 labelSpan["for"] = inputElem.id; OpenLayers.Element.addClass(labelSpan, "labelSpan olButton"); labelSpan._layer = layer.id; diff --git a/tests/Control/LayerSwitcher.html b/tests/Control/LayerSwitcher.html index 0094e77108..0b42257151 100644 --- a/tests/Control/LayerSwitcher.html +++ b/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(); + } From 465498b83a2f86c03df9486d18a3d017c872a429 Mon Sep 17 00:00:00 2001 From: Marc Jansen Date: Thu, 14 Feb 2013 09:10:18 +0100 Subject: [PATCH 3/3] Use blank.gif as WMS-mockup URL in test. --- tests/Control/LayerSwitcher.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Control/LayerSwitcher.html b/tests/Control/LayerSwitcher.html index 0b42257151..c81a77934a 100644 --- a/tests/Control/LayerSwitcher.html +++ b/tests/Control/LayerSwitcher.html @@ -204,13 +204,13 @@ layers: [ new OpenLayers.Layer.WMS( layername, - "http://example.com/" + "../../img/blank.gif" ), // add another layer with the same name, the generated id // must be different new OpenLayers.Layer.WMS( layername, - "http://example.com/" + "../../img/blank.gif" ) ] });