Fix + symbol appearing in changeset comments from external tools (#10766)

API.md

@@ -15,7 +15,7 @@ of iD (e.g. `https://ideditor-release.netlify.app`), the following parameters ar
   `{z}`/`{zoom}`, `{ty}` for flipped TMS-style Y coordinates, and `{switch:a,b,c}` for
   DNS multiplexing.<br/>
   _Example:_ `background=custom:https://tile.openstreetmap.org/{zoom}/{x}/{y}.png`
-* __`comment`__ - Prefills the changeset comment. Pass a url encoded string.<br/>
+* __`comment`__ - Prefills the changeset comment.<br/>
   _Example:_ `comment=CAR%20crisis%2C%20refugee%20areas%20in%20Cameroon`
 * __`disable_features`__ - Disables features in the list.<br/>
   _Example:_ `disable_features=water,service_roads,points,paths,boundaries`<br/>
@@ -24,7 +24,7 @@ of iD (e.g. `https://ideditor-release.netlify.app`), the following parameters ar
 * __`gpx`__ - A custom URL for loading a gpx track.  Specifying a `gpx` parameter will
   automatically enable the gpx layer for display.<br/>
   _Example:_ `gpx=https://gist.githubusercontent.com/answerquest/9445352b60ca5b44714675eae00f243a/raw/56a6343a29223318f4a697bfd16cbb2c3b8155ad/sample_boundary.gpx`
-* __`hashtags`__ - Prefills the changeset hashtags.  Pass a url encoded list of event
+* __`hashtags`__ - Prefills the changeset hashtags.  Pass a list of event
   hashtags separated by commas, semicolons, or spaces.  Leading '#' symbols are
   optional and will be added automatically. (Note that hashtag-like strings are
   automatically detected in the `comment`).<br/>
@@ -55,16 +55,19 @@ of iD (e.g. `https://ideditor-release.netlify.app`), the following parameters ar
 * __`presets`__ - A comma-separated list of preset IDs. These will be the only presets the user may select.<br/>
   _Example:_ `presets=building,highway/residential,highway/unclassified`
 * __`rtl=true`__ - Force iD into right-to-left mode (useful for testing).
-* __`source`__ - Prefills the changeset source. Pass a url encoded string.<br/>
+* __`source`__ - Prefills the changeset source.<br/>
   _Example:_ `source=Bing%3BMapillary`
-* __`validationDisable`__ - The issues identified by these types/subtypes will be disabled (i.e. Issues will not be shown at all). Each parameter value should contain a urlencoded, comma-separated list of type/subtype match rules.  An asterisk `*` may be used as a wildcard.<br/>
+* __`validationDisable`__ - The issues identified by these types/subtypes will be disabled (i.e. Issues will not be shown at all). Each parameter value should contain a comma-separated list of type/subtype match rules.  An asterisk `*` may be used as a wildcard.<br/>
   _Example:_ `validationDisable=crossing_ways/highway*,crossing_ways/tunnel*`
-* __`validationWarning`__ - The issues identified by these types/subtypes will be treated as warnings (i.e. Issues will be surfaced to the user but not block changeset upload). Each parameter value should contain a urlencoded, comma-separated list of type/subtype match rules.  An asterisk `*` may be used as a wildcard.<br/>
+* __`validationWarning`__ - The issues identified by these types/subtypes will be treated as warnings (i.e. Issues will be surfaced to the user but not block changeset upload). Each parameter value should contain a comma-separated list of type/subtype match rules.  An asterisk `*` may be used as a wildcard.<br/>
   _Example:_ `validationWarning=crossing_ways/highway*,crossing_ways/tunnel*`
-* __`validationError`__ - The issues identified by these types/subtypes will be treated as errors (i.e. Issues will be surfaced to the user but will block changeset upload). Each parameter value should contain a urlencoded, comma-separated list of type/subtype match rules.  An asterisk `*` may be used as a wildcard.<br/>
+* __`validationError`__ - The issues identified by these types/subtypes will be treated as errors (i.e. Issues will be surfaced to the user but will block changeset upload). Each parameter value should contain a comma-separated list of type/subtype match rules.  An asterisk `*` may be used as a wildcard.<br/>
   _Example:_ `validationError=crossing_ways/highway*,crossing_ways/tunnel*`
 * __`walkthrough=true`__ - Start the walkthrough automatically
 
+Pass these parameters as a `x-www-form-urlencoded` string in the _hash_ portion of the URL, similarly to how you how a _query_ string of an URL is typically constructed. Input strings have to be [percent encoded](https://en.wikipedia.org/wiki/Percent-encoding) (spaces can be represented either as `+` or `%20`), for example using [`URLSearchParams`](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/toString) in Javascript.
+
+
 ##### iD on openstreetmap.org (Rails Port)
 
 When constructing a URL to an instance of iD embedded on the [OpenStreetMap website](github.com/openstreetmap/openstreetmap-website/) (e.g. `https://www.openstreetmap.org/edit?editor=id`), the following parameters

CHANGELOG.md

@@ -51,6 +51,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
 * Preserve imagery offset during tile layer switching transition ([#10748])
 * Fix over-saturated map tiles near the border of the tile service's coverage area ([#10747], thanks [@hlfan])
 * Fix too dim markers of selected/hovered photo of some street level imagery layers ([#10755], thanks [@draunger])
+* Fix `+` symbol appearing in changeset comments from external tools ([#10766], thanks [@k-yle])
 #### :earth_asia: Localization
 #### :hourglass: Performance
 #### :mortar_board: Walkthrough / Help
@@ -63,6 +64,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
 [#10747]: https://github.com/openstreetmap/iD/issues/10747
 [#10748]: https://github.com/openstreetmap/iD/issues/10748
 [#10755]: https://github.com/openstreetmap/iD/issues/10755
+[#10766]: https://github.com/openstreetmap/iD/pull/10766
 [@hlfan]: https://github.com/hlfan
 [@Deeptanshu-sankhwar]: https://github.com/Deeptanshu-sankhwar
 [@draunger]: https://github.com/draunger

modules/util/util.js

@@ -358,31 +358,21 @@ 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) {
-            obj[parts[0]] = (null === parts[1]) ? '' : decodeURIComponent(parts[1]);
-        }
-        return obj;
-    }, {});
+    str = str.replace(/^[#?]{0,2}/, ''); // advance past any leading '?' or '#' characters
+    return Object.fromEntries(new URLSearchParams(str));
 }
 
 
-export function utilQsString(obj, noencode) {
-    // encode everything except special characters used in certain hash parameters:
-    // "/" in map states, ":", ",", {" and "}" in background
-    function softEncode(s) {
-        return encodeURIComponent(s).replace(/(%2F|%3A|%2C|%7B|%7D)/g, decodeURIComponent);
+export function utilQsString(obj, softEncode) {
+    let str = new URLSearchParams(obj).toString();
+    if (softEncode) {
+        // for better readability of URL hashes: optionally
+        // leave some special characters unescaped
+        //   "/" used in map state
+        //   ":", ",", {" and "}" used in background param
+        str = str.replace(/(%2F|%3A|%2C|%7B|%7D)/g, decodeURIComponent);
     }
-
-    return Object.keys(obj).sort().map(function(key) {
-        return encodeURIComponent(key) + '=' + (
-            noencode ? softEncode(obj[key]) : encodeURIComponent(obj[key]));
-    }).join('&');
+    return str;
 }
 
 

test/spec/behavior/hash.js

@@ -79,4 +79,13 @@ describe('iD.behaviorHash', function () {
             done();
         }, 600);
     });
+
+    it('accepts default changeset comment as hash parameter', function () {
+        window.location.hash = '#comment=foo+bar%20%2B1';
+        var container = d3.select(document.createElement('div'));
+        context = iD.coreContext().assetPath('../dist/').init().container(container);
+        iD.behaviorHash(context);
+        expect(context.defaultChangesetComment()).to.eql('foo bar +1');
+        hash.off();
+    });
 });

test/spec/util/util.js

@@ -80,31 +80,35 @@ describe('iD.util', function() {
 
     describe('utilStringQs', function() {
         it('splits a parameter string into k=v pairs', function() {
+            expect(iD.utilStringQs('')).to.eql({});
             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({});
+            expect(iD.utilStringQs('foo=bar baz')).to.eql({foo: 'bar baz'});
+            expect(iD.utilStringQs('foo=bar+baz')).to.eql({foo: 'bar baz'});
+            expect(iD.utilStringQs('foo=bar%20baz')).to.eql({foo: 'bar baz'});
         });
         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' });
+        });
+        it('supports both + and %20 for escaping spaces', function() {
+            expect(iD.utilStringQs('#?foo=a+b%20c')).to.eql({foo: 'a b c'});
             expect(iD.utilStringQs('#?')).to.eql({});
         });
     });
 
     it('utilQsString', function() {
+        expect(iD.utilQsString({})).to.eql('');
         expect(iD.utilQsString({ foo: 'bar' })).to.eql('foo=bar');
         expect(iD.utilQsString({ foo: 'bar', one: 2 })).to.eql('foo=bar&one=2');
-        expect(iD.utilQsString({})).to.eql('');
+        expect(iD.utilQsString({ foo: 'bar baz' })).to.be.oneOf(['foo=bar%20baz', 'foo=bar+baz']);
+        expect(iD.utilQsString({ foo: 'bar/baz' })).to.eql('foo=bar%2Fbaz');
+        expect(iD.utilQsString({ foo: 'bar/baz' }, true)).to.eql('foo=bar/baz');
     });
 
     describe('utilEditDistance', function() {