From 4a71aa6146821d6944e6ff83f8f3e0c46c7526ea Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Thu, 15 Mar 2018 00:57:41 -0400
Subject: [PATCH] Use safer field ids in classes, element ids, css selectors

Fixes issue with nested/namespaced fields, such as `maxspeed/advisory`,
in situations where would try to use a css selector or element id.
Can't use characters like '/' in a css selector.
---
 data/presets.yaml              | 5 +++++
 data/presets/fields.json       | 4 ++--
 data/presets/presets.json      | 5 +++++
 dist/locales/en.json           | 4 ++++
 modules/presets/field.js       | 2 ++
 modules/ui/field.js            | 4 ++--
 modules/ui/fields/check.js     | 4 ++--
 modules/ui/fields/combo.js     | 2 +-
 modules/ui/fields/input.js     | 2 +-
 modules/ui/fields/localized.js | 2 +-
 modules/ui/fields/maxspeed.js  | 4 ++--
 modules/ui/fields/textarea.js  | 2 +-
 modules/ui/fields/wikipedia.js | 2 +-
 modules/ui/form_fields.js      | 2 +-
 14 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/data/presets.yaml b/data/presets.yaml
index 5ffdf4ea2..68216a6f4 100644
--- a/data/presets.yaml
+++ b/data/presets.yaml
@@ -827,6 +827,11 @@ en:
         label: Speed Limit
         # maxspeed field placeholder
         placeholder: '40, 50, 60...'
+      maxspeed/advisory:
+        # 'maxspeed:advisory=*'
+        label: Advisory Speed Limit
+        # maxspeed/advisory field placeholder
+        placeholder: '40, 50, 60...'
       maxstay:
         # maxstay=*
         label: Max Stay
diff --git a/data/presets/fields.json b/data/presets/fields.json
index f85f292ea..f21bc4553 100644
--- a/data/presets/fields.json
+++ b/data/presets/fields.json
@@ -1139,7 +1139,7 @@
             "label": "Speed Limit",
             "placeholder": "40, 50, 60..."
         },
-        "maxspeed/advisory":{
+        "maxspeed/advisory": {
             "key": "maxspeed:advisory",
             "type": "maxspeed",
             "label": "Advisory Speed Limit",
@@ -2257,4 +2257,4 @@
             }
         }
     }
-}
+}
\ No newline at end of file
diff --git a/data/presets/presets.json b/data/presets/presets.json
index 5eb7fe335..6a1a08269 100644
--- a/data/presets/presets.json
+++ b/data/presets/presets.json
@@ -7290,6 +7290,7 @@
                 "ref_road_number",
                 "oneway",
                 "maxspeed",
+                "maxspeed/advisory",
                 "lanes",
                 "surface",
                 "structure",
@@ -7440,6 +7441,7 @@
                 "name",
                 "oneway",
                 "maxspeed",
+                "maxspeed/advisory",
                 "lanes",
                 "surface",
                 "maxheight",
@@ -7586,6 +7588,7 @@
                 "name",
                 "oneway",
                 "maxspeed",
+                "maxspeed/advisory",
                 "lanes",
                 "surface",
                 "structure",
@@ -7878,6 +7881,7 @@
                 "name",
                 "oneway",
                 "maxspeed",
+                "maxspeed/advisory",
                 "lanes",
                 "surface",
                 "structure",
@@ -8017,6 +8021,7 @@
                 "ref_road_number",
                 "oneway",
                 "maxspeed",
+                "maxspeed/advisory",
                 "lanes",
                 "surface",
                 "structure",
diff --git a/dist/locales/en.json b/dist/locales/en.json
index 8f0102fce..1c4833dce 100644
--- a/dist/locales/en.json
+++ b/dist/locales/en.json
@@ -2064,6 +2064,10 @@
                     "label": "Speed Limit",
                     "placeholder": "40, 50, 60..."
                 },
+                "maxspeed/advisory": {
+                    "label": "Advisory Speed Limit",
+                    "placeholder": "40, 50, 60..."
+                },
                 "maxstay": {
                     "label": "Max Stay"
                 },
diff --git a/modules/presets/field.js b/modules/presets/field.js
index 3fcc41613..b658c43a9 100644
--- a/modules/presets/field.js
+++ b/modules/presets/field.js
@@ -7,6 +7,8 @@ export function presetField(id, field) {
 
     field.id = id;
 
+    // for use in classes, element ids, css selectors
+    field.safeid = id.replace(/[^_a-zA-Z0-9\-]/g, '_');
 
     field.matchGeometry = function(geometry) {
         return !field.geometry || field.geometry === geometry;
diff --git a/modules/ui/field.js b/modules/ui/field.js
index 4936dd51a..756ab095e 100644
--- a/modules/ui/field.js
+++ b/modules/ui/field.js
@@ -96,14 +96,14 @@ export function uiField(context, presetField, entity, options) {
         // Enter
         var enter = container.enter()
             .append('div')
-            .attr('class', function(d) { return 'form-field form-field-' + d.id; })
+            .attr('class', function(d) { return 'form-field form-field-' + d.safeid; })
             .classed('nowrap', !options.wrap);
 
         if (options.wrap) {
             var label = enter
                 .append('label')
                 .attr('class', 'form-label')
-                .attr('for', function(d) { return 'preset-input-' + d.id; })
+                .attr('for', function(d) { return 'preset-input-' + d.safeid; })
                 .text(function(d) { return d.label(); });
 
             var wrap = label
diff --git a/modules/ui/fields/check.js b/modules/ui/fields/check.js
index eb60d6646..f7300c598 100644
--- a/modules/ui/fields/check.js
+++ b/modules/ui/fields/check.js
@@ -103,7 +103,7 @@ export function uiFieldCheck(field, context) {
             .append('input')
             .property('indeterminate', field.type !== 'defaultCheck')
             .attr('type', 'checkbox')
-            .attr('id', 'preset-input-' + field.id);
+            .attr('id', 'preset-input-' + field.safeid);
 
         enter
             .append('span')
@@ -113,7 +113,7 @@ export function uiFieldCheck(field, context) {
         if (field.type === 'onewayCheck') {
             enter
                 .append('a')
-                .attr('id', 'preset-input-' + field.id + '-reverser')
+                .attr('id', 'preset-input-' + field.safeid + '-reverser')
                 .attr('class', 'reverser button' + (reverserHidden() ? ' hide' : ''))
                 .attr('href', '#')
                 .append('span')
diff --git a/modules/ui/fields/combo.js b/modules/ui/fields/combo.js
index 875fc6393..3da0e37ad 100644
--- a/modules/ui/fields/combo.js
+++ b/modules/ui/fields/combo.js
@@ -291,7 +291,7 @@ export function uiFieldCombo(field, context) {
         input = input.enter()
             .append('input')
             .attr('type', 'text')
-            .attr('id', 'preset-input-' + field.id)
+            .attr('id', 'preset-input-' + field.safeid)
             .call(utilNoAuto)
             .call(initCombo, selection)
             .merge(input);
diff --git a/modules/ui/fields/input.js b/modules/ui/fields/input.js
index b5a49a6c7..7a33d7071 100644
--- a/modules/ui/fields/input.js
+++ b/modules/ui/fields/input.js
@@ -27,7 +27,7 @@ export function uiFieldText(field, context) {
 
 
     function i(selection) {
-        var fieldId = 'preset-input-' + field.id;
+        var fieldId = 'preset-input-' + field.safeid;
 
         input = selection.selectAll('input')
             .data([0]);
diff --git a/modules/ui/fields/localized.js b/modules/ui/fields/localized.js
index 35ea06f3a..5688a17e8 100644
--- a/modules/ui/fields/localized.js
+++ b/modules/ui/fields/localized.js
@@ -39,7 +39,7 @@ export function uiFieldLocalized(field, context) {
         input = input.enter()
             .append('input')
             .attr('type', 'text')
-            .attr('id', 'preset-input-' + field.id)
+            .attr('id', 'preset-input-' + field.safeid)
             .attr('class', 'localized-main')
             .attr('placeholder', field.placeholder())
             .call(utilNoAuto)
diff --git a/modules/ui/fields/maxspeed.js b/modules/ui/fields/maxspeed.js
index b304aab1b..fb0e5c2ed 100644
--- a/modules/ui/fields/maxspeed.js
+++ b/modules/ui/fields/maxspeed.js
@@ -33,13 +33,13 @@ export function uiFieldMaxspeed(field, context) {
             .container(context.container())
             .data(['km/h', 'mph'].map(comboValues));
 
-        input = selection.selectAll('#preset-input-' + field.id)
+        input = selection.selectAll('#preset-input-' + field.safeid)
             .data([0]);
 
         input = input.enter()
             .append('input')
             .attr('type', 'text')
-            .attr('id', 'preset-input-' + field.id)
+            .attr('id', 'preset-input-' + field.safeid)
             .attr('placeholder', field.placeholder())
             .call(utilNoAuto)
             .call(combobox)
diff --git a/modules/ui/fields/textarea.js b/modules/ui/fields/textarea.js
index 08d057cc3..a630292f3 100644
--- a/modules/ui/fields/textarea.js
+++ b/modules/ui/fields/textarea.js
@@ -20,7 +20,7 @@ export function uiFieldTextarea(field) {
 
         input = input.enter()
             .append('textarea')
-            .attr('id', 'preset-input-' + field.id)
+            .attr('id', 'preset-input-' + field.safeid)
             .attr('placeholder', field.placeholder() || t('inspector.unknown'))
             .attr('maxlength', 255)
             .call(utilNoAuto)
diff --git a/modules/ui/fields/wikipedia.js b/modules/ui/fields/wikipedia.js
index a292a7e67..da2a8b83e 100644
--- a/modules/ui/fields/wikipedia.js
+++ b/modules/ui/fields/wikipedia.js
@@ -91,7 +91,7 @@ export function uiFieldWikipedia(field, context) {
             .append('input')
             .attr('type', 'text')
             .attr('class', 'wiki-title')
-            .attr('id', 'preset-input-' + field.id)
+            .attr('id', 'preset-input-' + field.safeid)
             .call(utilNoAuto)
             .merge(title);
 
diff --git a/modules/ui/form_fields.js b/modules/ui/form_fields.js
index b6ca78cc7..e2c6de218 100644
--- a/modules/ui/form_fields.js
+++ b/modules/ui/form_fields.js
@@ -37,7 +37,7 @@ export function uiFormFields(context) {
         // Enter
         var enter = fields.enter()
             .append('div')
-            .attr('class', function(d) { return 'wrap-form-field wrap-form-field-' + d.id; });
+            .attr('class', function(d) { return 'wrap-form-field wrap-form-field-' + d.safeid; });
 
         // Update
         fields = fields