Merge branch 'develop' into way-reverse-dont-change-red-turn-direction

test/spec/actions/circularize.js

@@ -278,10 +278,11 @@ describe('iD.actionCircularize', function () {
         var graph = iD.coreGraph([
                 iD.osmNode({id: 'a', loc: [0, 0]}),
                 iD.osmNode({id: 'b', loc: [0, 2]}),
-                iD.osmWay({id: '-', nodes: ['a', 'b', 'a']})
+                iD.osmNode({id: 'c', loc: [2, 0]}),
+                iD.osmWay({id: '-', nodes: ['a', 'b', 'c', 'a']})
             ]);
 
-        expect(area('-', graph)).to.eql(0);
+        expect(area('-', graph)).to.eql(2);
 
         graph = iD.actionCircularize('-', projection)(graph);
 

test/spec/actions/split.js

@@ -511,6 +511,26 @@ describe('iD.actionSplit', function () {
             expect(g4.entity('-').nodes).to.eql(['b', 'c', 'd']);
             expect(g4.entity('=').nodes).to.eql(['d', 'a', 'b']);
         });
+
+        it('splits a closed way at the given points', function () {
+            //
+            // Situation:
+            //    a ---- b
+            //    |      |
+            //    d ---- c
+            //
+            var graph = iD.coreGraph([
+                iD.osmNode({ id: 'a', loc: [0, 1] }),
+                iD.osmNode({ id: 'b', loc: [1, 1] }),
+                iD.osmNode({ id: 'c', loc: [1, 0] }),
+                iD.osmNode({ id: 'd', loc: [0, 0] }),
+                iD.osmWay({ id: '-', nodes: ['a', 'b', 'c', 'd', 'a']})
+            ]);
+
+            var g1 = iD.actionSplit(['a', 'b'], ['='])(graph);
+            expect(g1.entity('-').nodes).to.eql(['b', 'c', 'd', 'a']);
+            expect(g1.entity('=').nodes).to.eql(['a', 'b']);
+        });
     });
 
 

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/osm/way.js

@@ -305,7 +305,6 @@ describe('iD.osmWay', function() {
 
         it('returns true when the way has tag oneway=yes', function() {
             expect(iD.osmWay({tags: { oneway: 'yes' }}).isOneWay(), 'oneway yes').to.be.true;
-            expect(iD.osmWay({tags: { oneway: '1' }}).isOneWay(), 'oneway 1').to.be.true;
             expect(iD.osmWay({tags: { oneway: '-1' }}).isOneWay(), 'oneway -1').to.be.true;
         });
 
@@ -473,6 +472,10 @@ describe('iD.osmWay', function() {
             expect(iD.osmWay({nodes: ['a', 'b']}).isDegenerate()).to.equal(false);
         });
 
+        it('returns true for a linear way that doubles back on itself', function () {
+            expect(iD.osmWay({nodes: ['a', 'b', 'a']}).isDegenerate()).to.equal(true);
+        });
+
         it('returns true for an area with zero, one, or two unique nodes', function () {
             expect(iD.osmWay({tags: {area: 'yes'}, nodes: []}).isDegenerate()).to.equal(true);
             expect(iD.osmWay({tags: {area: 'yes'}, nodes: ['a', 'a']}).isDegenerate()).to.equal(true);

test/spec/services/osm.js

@@ -163,14 +163,14 @@ describe('iD.serviceOsm', function () {
             });
         });
 
-        it('retries an authenticated call unauthenticated if 400 Bad Request', function (done) {
+        it('retries an authenticated call unauthenticated if 401 Unauthorized', function (done) {
             fetchMock.mock('https://www.openstreetmap.org' + path, {
                 body: response,
                 status: 200,
                 headers: { 'Content-Type': 'application/json' }
             });
             serverXHR.respondWith('GET', 'https://www.openstreetmap.org' + path,
-                [400, { 'Content-Type': 'text/plain' }, 'Bad Request']);
+                [401, { 'Content-Type': 'text/plain' }, 'Unauthorized']);
 
             login();
 

test/spec/services/osm_wikibase.js

@@ -331,4 +331,75 @@ describe('iD.serviceOsmWikibase', function () {
     });
   });
 
+  describe('linkifyWikiText', () => {
+    it('handles normal text', () => {
+      const main = document.createElement('main');
+      d3.select(main).call(iD.serviceOsmWikibase.linkifyWikiText('hello'));
+
+      expect(main.innerHTML).toBe('<span>hello</span>');
+      expect(main.textContent).toBe('hello');
+    });
+
+    it('prevents XSS attacks', () => {
+      const main = document.createElement('main');
+      d3.select(main).call(iD.serviceOsmWikibase.linkifyWikiText('123 <script>bad</script> 456'));
+
+      expect(main.innerHTML).toBe('<span>123 &lt;script&gt;bad&lt;/script&gt; 456</span>');
+      expect(main.textContent).toBe('123 <script>bad</script> 456');
+    });
+
+    it('linkifies the tag: and key: syntax', () => {
+      const main = document.createElement('main');
+      d3.select(main).call(iD.serviceOsmWikibase.linkifyWikiText('use tag:natural=water with key:water instead'));
+
+      expect(main.innerHTML).toBe([
+        '<span>use </span>',
+        '<a href="https://wiki.openstreetmap.org/wiki/Tag:natural=water" target="_blank" rel="noreferrer"><code>natural=water</code></a>',
+        '<span> with </span>',
+        '<a href="https://wiki.openstreetmap.org/wiki/Key:water" target="_blank" rel="noreferrer"><code>water=*</code></a>',
+        '<span> instead</span>'
+      ].join(''));
+      expect(main.textContent).toBe('use natural=water with water=* instead');
+    });
+
+    it('works if the string is 100% a link', () => {
+      const main = document.createElement('main');
+      d3.select(main).call(iD.serviceOsmWikibase.linkifyWikiText('tag:natural=water'));
+
+      expect(main.innerHTML).toBe([
+        '<a href="https://wiki.openstreetmap.org/wiki/Tag:natural=water" target="_blank" rel="noreferrer"><code>natural=water</code></a>',
+      ].join(''));
+      expect(main.textContent).toBe('natural=water');
+    });
+
+    it('works if the link is the first part of the string', () => {
+      const main = document.createElement('main');
+      d3.select(main).call(iD.serviceOsmWikibase.linkifyWikiText('tag:craft=sailmaker is better'));
+
+      expect(main.innerHTML).toBe([
+        '<a href="https://wiki.openstreetmap.org/wiki/Tag:craft=sailmaker" target="_blank" rel="noreferrer"><code>craft=sailmaker</code></a>',
+        '<span> is better</span>'
+      ].join(''));
+      expect(main.textContent).toBe('craft=sailmaker is better');
+    });
+
+    it('works if the link is the last part of the string', () => {
+      const main = document.createElement('main');
+      d3.select(main).call(iD.serviceOsmWikibase.linkifyWikiText('prefer tag:craft=sailmaker'));
+
+      expect(main.innerHTML).toBe([
+        '<span>prefer </span>',
+        '<a href="https://wiki.openstreetmap.org/wiki/Tag:craft=sailmaker" target="_blank" rel="noreferrer"><code>craft=sailmaker</code></a>',
+      ].join(''));
+      expect(main.textContent).toBe('prefer craft=sailmaker');
+    });
+
+    it('handles empty strings', () => {
+      const main = document.createElement('main');
+      d3.select(main).call(iD.serviceOsmWikibase.linkifyWikiText(''));
+
+      expect(main.innerHTML).toBe('');
+      expect(main.textContent).toBe('');
+    });
+  });
 });

test/spec/ui/fields/directional_combo.js

@@ -0,0 +1,111 @@
+describe('iD.uiFieldDirectionalCombo', () => {
+    /** @type {iD.Context} */
+    let context;
+    /** @type {import("d3-selection").Selection} */
+    let selection;
+
+    beforeEach(() => {
+        context = iD.coreContext().assetPath('../dist/').init();
+        selection = d3.select(document.createElement('div'));
+    });
+
+    describe.each(['cycleway', 'cycleway:both'])('preset uses %s', (commonKey) => {
+        /** if commonKey ends with :both, this is the key without :both. and vice-verca */
+        const otherCommonKey = commonKey.endsWith(':both')
+            ? commonKey.replace(/:both$/, '')
+            : `${commonKey}:both`;
+
+        const field = iD.presetField('name', {
+            key: commonKey,
+            keys: ['cycleway:left', 'cycleway:right'],
+        });
+
+        it('populates the left/right fields using :left & :right', () => {
+            const instance = iD.uiFieldDirectionalCombo(field, context);
+            selection.call(instance);
+            instance.tags({ 'cycleway:left': 'lane' });
+
+            expect(selection.selectAll('input').nodes()).toHaveLength(2);
+            const [left, right] = selection.selectAll('input').nodes();
+            expect(left.value).toBe('lane');
+            expect(right.value).toBe('');
+        });
+
+        it('populates the left/right fields using :both', () => {
+            const instance = iD.uiFieldDirectionalCombo(field, context);
+            selection.call(instance);
+            instance.tags({ 'cycleway:both': 'lane' });
+
+            expect(selection.selectAll('input').nodes()).toHaveLength(2);
+            const [left, right] = selection.selectAll('input').nodes();
+            expect(left.value).toBe('lane');
+            expect(right.value).toBe('lane');
+        });
+
+        it('populates the left/right fields using the unprefixed tag', () => {
+            const instance = iD.uiFieldDirectionalCombo(field, context);
+            selection.call(instance);
+            instance.tags({ cycleway: 'lane' });
+
+            expect(selection.selectAll('input').nodes()).toHaveLength(2);
+            const [left, right] = selection.selectAll('input').nodes();
+            expect(left.value).toBe('lane');
+            expect(right.value).toBe('lane');
+        });
+
+        it(`setting left & right to the same value will use the ${commonKey}`, () => {
+            const instance = iD.uiFieldDirectionalCombo(field, context);
+            selection.call(instance);
+            const tags = { 'cycleway:left': 'lane', 'cycleway:right': 'shoulder' };
+            instance.tags(tags);
+
+            const onChange = vi.fn();
+            instance.on('change', v => onChange(v(tags)));
+
+            expect(selection.selectAll('input').nodes()).toHaveLength(2);
+            const [left, right] = selection.selectAll('input').nodes();
+            expect(left.value).toBe('lane');
+            expect(right.value).toBe('shoulder');
+
+
+            left.value = 'shoulder';
+            d3.select(left).dispatch('change');
+
+            expect(onChange).toHaveBeenCalledTimes(1);
+            expect(onChange).toHaveBeenCalledWith({ [commonKey]: 'shoulder' });
+        });
+
+        it(`can read the value from ${otherCommonKey}, but writes to ${commonKey}`, () => {
+            const instance = iD.uiFieldDirectionalCombo(field, context);
+            selection.call(instance);
+            let tags = { [otherCommonKey]: 'lane' };
+            instance.tags(tags);
+
+            const onChange = vi.fn();
+            instance.on('change', v => onChange(tags = v(tags)));
+
+            expect(selection.selectAll('input').nodes()).toHaveLength(2);
+            const [left, right] = selection.selectAll('input').nodes();
+            expect(left.value).toBe('lane');
+            expect(right.value).toBe('lane');
+
+
+            left.value = 'shoulder';
+            d3.select(left).dispatch('change');
+
+            expect(onChange).toHaveBeenCalledTimes(1);
+            expect(onChange).toHaveBeenNthCalledWith(1, {
+                'cycleway:left': 'shoulder', // left was updated
+                'cycleway:right': 'lane',
+            });
+
+            right.value = 'shoulder';
+            d3.select(right).dispatch('change');
+
+            expect(onChange).toHaveBeenCalledTimes(2);
+            expect(onChange).toHaveBeenNthCalledWith(2, {
+                [commonKey]: 'shoulder', // now left & right have been updated
+            });
+        });
+    });
+});

test/spec/util/object.js

@@ -7,3 +7,16 @@ describe('iD.utilObjectOmit', function() {
     });
 });
 
+
+describe('iD.utilCheckTagDictionary', () => {
+    it('can search a standard tag-dictionary', () => {
+        expect(iD.utilCheckTagDictionary({}, iD.osmPavedTags)).toBeUndefined();
+        expect(iD.utilCheckTagDictionary({ surface: 'asphalt' }, iD.osmPavedTags)).toBe(true);
+    });
+
+    it('works for falsy values', () => {
+        const dictionary = { surface: { paved: 0 } };
+        expect(iD.utilCheckTagDictionary({}, dictionary)).toBeUndefined();
+        expect(iD.utilCheckTagDictionary({ surface: 'paved' }, dictionary)).toBe(0);
+    });
+});

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() {

test/spec/validations/crossing_ways.js

@@ -202,6 +202,60 @@ describe('iD.validations.crossing_ways', function () {
         expect(issues).to.have.lengthOf(0);
     });
 
+    it('ignores a routable aeroway crossing a non-routable aeroway', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'taxiway' }, { aeroway: 'aerodrome' });
+        const issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('ignores an aeroway crossing a road tunnel', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway' }, { highway: 'trunk', tunnel: 'yes', layer: '-1' });
+        const issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('ignores an aeroway crossing a road bridge', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway' }, { highway: 'trunk', bridge: 'yes', layer: '1' });
+        const issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('ignores an aeroway crossing a rail tunnel', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway' }, { railway: 'track', tunnel: 'yes', layer: '-1' });
+        const issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('ignores an aeroway crossing a rail bridge', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway' }, { railway: 'track', bridge: 'yes', layer: '1' });
+        const issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('ignores an aeroway bridge crossing a road', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway', bridge: 'yes', layer: '2' }, { highway: 'trunk', layer: '1' });
+        const issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('ignores an aeroway bridge crossing a railway', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway', bridge: 'yes', layer: '1' }, { railway: 'track' });
+        const issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('ignores an aeroway crossing a culvert', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'taxiway' }, { waterway: 'ditch', tunnel: 'culvert', layer: -1 });
+        const issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('ignores an aeroway crossing a building on a different layer', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway' }, { building: 'yes', layer: '0.5' });
+        const issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
     // warning crossing cases between ways
     it('flags road crossing road', function() {
         createWaysWithOneCrossingPoint({ highway: 'residential' }, { highway: 'residential' });
@@ -453,4 +507,38 @@ describe('iD.validations.crossing_ways', function () {
         verifySingleCrossingIssue(validate(), {});
     });
 
+    it('flags an aeroway crosing another aeroway', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway' }, { aeroway: 'taxiway' });
+        verifySingleCrossingIssue(validate(), {});
+    });
+
+    it('flags an aeroway crosing a major road', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway' }, { highway: 'motorway' });
+        verifySingleCrossingIssue(validate(), { aeroway: 'aircraft_crossing' });
+    });
+
+    it('flags an aeroway crosing a service road', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway' }, { highway: 'service' });
+        verifySingleCrossingIssue(validate(), {});
+    });
+
+    it('flags an aeroway crosing a path', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway' }, { highway: 'corridor' });
+        verifySingleCrossingIssue(validate(), {});
+    });
+
+    it('flags an aeroway crosing a railway', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'taxiway' }, { railway: 'disused' });
+        verifySingleCrossingIssue(validate(), { aeroway: 'aircraft_crossing', railway: 'level_crossing' });
+    });
+
+    it('flags an aeroway crosing a waterway', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway' }, { waterway: 'canal' });
+        verifySingleCrossingIssue(validate(), null);
+    });
+
+    it('flags an aeroway crosing a building', function() {
+        createWaysWithOneCrossingPoint({ aeroway: 'runway' }, { building: 'hangar' });
+        verifySingleCrossingIssue(validate(), null);
+    });
 });

test/spec/validations/osm_api_limits.js

@@ -0,0 +1,96 @@
+describe('iD.validations.osm_api_limits', function () {
+    var context;
+
+    beforeEach(function() {
+        iD.services.osm = { maxWayNodes: function() { return 10; } };
+        context = iD.coreContext().assetPath('../dist/').init();
+        context.surface = () => d3.select('#nop'); // mock with NOP
+    });
+
+    function createWay(numNodes) {
+        var nodes = [];
+        for (var i = 0; i < numNodes; i++) {
+            nodes.push(iD.osmNode({ id: 'n-' + i, loc: [i, i]}));
+        }
+        var w = iD.osmWay({id: 'w-1', tags: {},
+            nodes: nodes.map(function(n) { return n.id; }) });
+
+        context.perform.apply(null, nodes
+            .map(function(n) { return iD.actionAddEntity(n); })
+            .concat(iD.actionAddEntity(w))
+        );
+    }
+
+    function validate() {
+        var validator = iD.validationOsmApiLimits(context);
+        var changes = context.history().changes();
+        var entities = changes.modified.concat(changes.created);
+        var issues = [];
+        entities.forEach(function(entity) {
+            issues = issues.concat(validator(entity, context.graph()));
+        });
+        return issues;
+    }
+
+    it('has no errors on init', function() {
+        var issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('flags way with more than the maximum number of allowed nodes', function() {
+        createWay(12);
+        var issues = validate();
+        expect(issues).to.have.lengthOf(1);
+        var issue = issues[0];
+        expect(issue.type).to.eql('osm_api_limits');
+        expect(issue.subtype).to.eql('exceededMaxWayNodes');
+        expect(issue.severity).to.eql('error');
+        expect(issue.entityIds).to.have.lengthOf(1);
+        expect(issue.entityIds[0]).to.eql('w-1');
+
+        var fixes = issue.fixes(context);
+        expect(fixes).to.have.lengthOf(1);
+        fixes[0].onClick(context);
+        issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('can fix an extreme case', function() {
+        createWay(33);
+        var issues = validate();
+        expect(issues).to.have.lengthOf(1);
+        var issue = issues[0];
+
+        var fixes = issue.fixes(context);
+        expect(fixes).to.have.lengthOf(1);
+        fixes[0].onClick(context);
+        issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('fix a simple case at an intersection vertex', function() {
+        createWay(12);
+
+        var n2 = iD.osmNode({id: 'n-0', loc: [0,0]});
+        var n1 = iD.osmNode({id: 'n-8', loc: [8,8]});
+        var w = iD.osmWay({id: 'w-2', nodes: ['n-0', 'n-8'], tags: {}});
+
+        context.perform(
+            iD.actionAddEntity(n1),
+            iD.actionAddEntity(n2),
+            iD.actionAddEntity(w)
+        );
+
+        var issues = validate();
+        expect(issues).to.have.lengthOf(1);
+        var issue = issues[0];
+
+        var fixes = issue.fixes(context);
+        expect(fixes).to.have.lengthOf(1);
+        fixes[0].onClick(context);
+        issues = validate();
+        expect(issues).to.have.lengthOf(0);
+
+        context.graph().entity('w-1').nodes.length === 8;
+    });
+});