From dc7fba4bf8eb1dcad4bb16313a10d64f779bfdb0 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 20 Feb 2020 17:09:54 -0500 Subject: [PATCH] Have utilStringQs advance past any leading '?' or '#' characters This lets us remove a bunch of substring(1) and +1 from the code. --- modules/behavior/hash.js | 4 ++-- modules/renderer/background.js | 32 +++++++++++++++--------------- modules/renderer/features.js | 14 ++++++------- modules/renderer/photos.js | 14 ++++++------- modules/util/util.js | 4 ++++ test/spec/services/nominatim.js | 2 +- test/spec/services/osm_wikibase.js | 2 +- test/spec/services/taginfo.js | 2 +- test/spec/spec_helpers.js | 2 ++ test/spec/util/util.js | 23 ++++++++++++++++++--- 10 files changed, 61 insertions(+), 38 deletions(-) diff --git a/modules/behavior/hash.js b/modules/behavior/hash.js index 853b8f42c..6b3937654 100644 --- a/modules/behavior/hash.js +++ b/modules/behavior/hash.js @@ -39,7 +39,7 @@ export function behaviorHash(context) { var center = map.center(); var zoom = map.zoom(); var precision = Math.max(0, Math.ceil(Math.log(zoom) / Math.LN2)); - var q = utilObjectOmit(utilStringQs(window.location.hash.substring(1)), + var q = utilObjectOmit(utilStringQs(window.location.hash), ['comment', 'source', 'hashtags', 'walkthrough'] ); var newParams = {}; @@ -91,7 +91,7 @@ export function behaviorHash(context) { .on('hashchange.hash', hashchange); if (window.location.hash) { - var q = utilStringQs(window.location.hash.substring(1)); + var q = utilStringQs(window.location.hash); if (q.id) { context.zoomToEntity(q.id.split(',')[0], !q.map); diff --git a/modules/renderer/background.js b/modules/renderer/background.js index e9e5ecdfc..9fd07463a 100644 --- a/modules/renderer/background.js +++ b/modules/renderer/background.js @@ -203,7 +203,7 @@ export function rendererBackground(context) { const EPSILON = 0.01; const x = +meters[0].toFixed(2); const y = +meters[1].toFixed(2); - let q = utilStringQs(window.location.hash.substring(1)); + let hash = utilStringQs(window.location.hash); let id = currSource.id; if (id === 'custom') { @@ -211,25 +211,25 @@ export function rendererBackground(context) { } if (id) { - q.background = id; + hash.background = id; } else { - delete q.background; + delete hash.background; } if (o) { - q.overlays = o; + hash.overlays = o; } else { - delete q.overlays; + delete hash.overlays; } if (Math.abs(x) > EPSILON || Math.abs(y) > EPSILON) { - q.offset = `${x},${y}`; + hash.offset = `${x},${y}`; } else { - delete q.offset; + delete hash.offset; } if (!window.mocha) { - window.location.replace('#' + utilQsString(q, true)); + window.location.replace('#' + utilQsString(hash, true)); } let imageryUsed = []; @@ -444,9 +444,9 @@ export function rendererBackground(context) { return geoExtent([params[2], params[1]]); // lon,lat } - const q = utilStringQs(window.location.hash.substring(1)); - const requested = q.background || q.layer; - let extent = parseMapParams(q.map); + const hash = utilStringQs(window.location.hash); + const requested = hash.background || hash.layer; + let extent = parseMapParams(hash.map); ensureImageryIndex() .then(imageryIndex => { @@ -479,7 +479,7 @@ export function rendererBackground(context) { background.toggleOverlayLayer(locator); } - const overlays = (q.overlays || '').split(','); + const overlays = (hash.overlays || '').split(','); overlays.forEach(overlay => { overlay = background.findSource(overlay); if (overlay) { @@ -487,15 +487,15 @@ export function rendererBackground(context) { } }); - if (q.gpx) { // todo: move elsewhere - this doesn't belong in background + if (hash.gpx) { // todo: move elsewhere - this doesn't belong in background const gpx = context.layers().layer('data'); if (gpx) { - gpx.url(q.gpx, '.gpx'); + gpx.url(hash.gpx, '.gpx'); } } - if (q.offset) { - const offset = q.offset + if (hash.offset) { + const offset = hash.offset .replace(/;/g, ',') .split(',') .map(n => !isNaN(n) && n); diff --git a/modules/renderer/features.js b/modules/renderer/features.js index fba1b2268..e908df604 100644 --- a/modules/renderer/features.js +++ b/modules/renderer/features.js @@ -64,14 +64,14 @@ export function rendererFeatures(context) { function update() { if (!window.mocha) { - var q = utilStringQs(window.location.hash.substring(1)); + var hash = utilStringQs(window.location.hash); var disabled = features.disabled(); if (disabled.length) { - q.disable_features = disabled.join(','); + hash.disable_features = disabled.join(','); } else { - delete q.disable_features; + delete hash.disable_features; } - window.location.replace('#' + utilQsString(q, true)); + window.location.replace('#' + utilQsString(hash, true)); context.storage('disabled-features', disabled.join(',')); } _hidden = features.hidden(); @@ -579,9 +579,9 @@ export function rendererFeatures(context) { storageDisabled.forEach(features.disable); } - var q = utilStringQs(window.location.hash.substring(1)); - if (q.disable_features) { - var hashDisabled = q.disable_features.replace(/;/g, ',').split(','); + var hash = utilStringQs(window.location.hash); + if (hash.disable_features) { + var hashDisabled = hash.disable_features.replace(/;/g, ',').split(','); hashDisabled.forEach(features.disable); } }; diff --git a/modules/renderer/photos.js b/modules/renderer/photos.js index f1751d03b..62eb89ce9 100644 --- a/modules/renderer/photos.js +++ b/modules/renderer/photos.js @@ -15,18 +15,18 @@ export function rendererPhotos(context) { function updateStorage() { if (window.mocha) return; - var q = utilStringQs(window.location.hash.substring(1)); + var hash = utilStringQs(window.location.hash); var enabled = context.layers().all().filter(function(d) { return _layerIDs.indexOf(d.id) !== -1 && d.layer && d.layer.supported() && d.layer.enabled(); }).map(function(d) { return d.id; }); if (enabled.length) { - q.photo_overlay = enabled.join(','); + hash.photo_overlay = enabled.join(','); } else { - delete q.photo_overlay; + delete hash.photo_overlay; } - window.location.replace('#' + utilQsString(q, true)); + window.location.replace('#' + utilQsString(hash, true)); } photos.overlayLayerIDs = function() { @@ -73,9 +73,9 @@ export function rendererPhotos(context) { }; photos.init = function() { - var q = utilStringQs(window.location.hash.substring(1)); - if (q.photo_overlay) { - var hashOverlayIDs = q.photo_overlay.replace(/;/g, ',').split(','); + var hash = utilStringQs(window.location.hash); + if (hash.photo_overlay) { + var hashOverlayIDs = hash.photo_overlay.replace(/;/g, ',').split(','); hashOverlayIDs.forEach(function(id) { var layer = context.layers().layer(id); if (layer) layer.enabled(true); diff --git a/modules/util/util.js b/modules/util/util.js index a7ea8675e..84204680d 100644 --- a/modules/util/util.js +++ b/modules/util/util.js @@ -318,6 +318,10 @@ export function utilCombinedTags(entityIDs, graph) { export function utilStringQs(str) { + var i = 0; // advance past any leading '?' or '#' characters + while (i < str.length && (str[i] === '?' || str[i] === '#')) i++; + str = str.slice(i); + return str.split('&').reduce(function(obj, pair){ var parts = pair.split('='); if (parts.length === 2) { diff --git a/test/spec/services/nominatim.js b/test/spec/services/nominatim.js index 8773018a9..f28982cd0 100644 --- a/test/spec/services/nominatim.js +++ b/test/spec/services/nominatim.js @@ -21,7 +21,7 @@ describe('iD.serviceNominatim', function() { }); function query(url) { - return iD.utilStringQs(url.substring(url.indexOf('?') + 1)); + return iD.utilStringQs(url.substring(url.indexOf('?'))); } diff --git a/test/spec/services/osm_wikibase.js b/test/spec/services/osm_wikibase.js index ae05015d3..58689647f 100644 --- a/test/spec/services/osm_wikibase.js +++ b/test/spec/services/osm_wikibase.js @@ -21,7 +21,7 @@ describe('iD.serviceOsmWikibase', function () { function query(url) { - return iD.utilStringQs(url.substring(url.indexOf('?') + 1)); + return iD.utilStringQs(url.substring(url.indexOf('?'))); } function adjust(params, data) { diff --git a/test/spec/services/taginfo.js b/test/spec/services/taginfo.js index 0f6843e88..80d46cd7f 100644 --- a/test/spec/services/taginfo.js +++ b/test/spec/services/taginfo.js @@ -31,7 +31,7 @@ describe('iD.serviceTaginfo', function() { }); function query(url) { - return iD.utilStringQs(url.substring(url.indexOf('?') + 1)); + return iD.utilStringQs(url.substring(url.indexOf('?'))); } diff --git a/test/spec/spec_helpers.js b/test/spec/spec_helpers.js index 490275e86..0dfeebe5e 100644 --- a/test/spec/spec_helpers.js +++ b/test/spec/spec_helpers.js @@ -7,6 +7,8 @@ for (var k in iD.services) { delete iD.services[k]; } // Run without data for speed (tests which need data can set it up themselves) +// Initializing `coreContext` will try loading the English locale strings: +iD.data.locale_en = { en: {} }; // Initializing `coreContext` initializes `_background`, which tries loading: iD.data.imagery = []; // Initializing `coreContext` initializes `_presets`, which tries loading: diff --git a/test/spec/util/util.js b/test/spec/util/util.js index 63b999955..b71736ac8 100644 --- a/test/spec/util/util.js +++ b/test/spec/util/util.js @@ -79,9 +79,26 @@ describe('iD.util', function() { }); it('utilStringQs', function() { - expect(iD.utilStringQs('foo=bar')).to.eql({foo: 'bar'}); - expect(iD.utilStringQs('foo=bar&one=2')).to.eql({foo: 'bar', one: '2' }); - expect(iD.utilStringQs('')).to.eql({}); + it('splits a parameter string into k=v pairs', function() { + expect(iD.utilStringQs('foo=bar')).to.eql({foo: 'bar'}); + expect(iD.utilStringQs('foo=bar&one=2')).to.eql({foo: 'bar', one: '2' }); + expect(iD.utilStringQs('')).to.eql({}); + }); + it('trims leading # if present', function() { + expect(iD.utilStringQs('#foo=bar')).to.eql({foo: 'bar'}); + expect(iD.utilStringQs('#foo=bar&one=2')).to.eql({foo: 'bar', one: '2' }); + expect(iD.utilStringQs('#')).to.eql({}); + }); + it('trims leading ? if present', function() { + expect(iD.utilStringQs('?foo=bar')).to.eql({foo: 'bar'}); + expect(iD.utilStringQs('?foo=bar&one=2')).to.eql({foo: 'bar', one: '2' }); + expect(iD.utilStringQs('?')).to.eql({}); + }); + it('trims leading #? if present', function() { + expect(iD.utilStringQs('#?foo=bar')).to.eql({foo: 'bar'}); + expect(iD.utilStringQs('#?foo=bar&one=2')).to.eql({foo: 'bar', one: '2' }); + expect(iD.utilStringQs('#?')).to.eql({}); + }); }); it('utilQsString', function() {