From 286c09366aadb29165c575300dab02a38fc6e503 Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Thu, 4 Jan 2018 15:55:53 -0500
Subject: [PATCH 01/10] Remove unused var (eslint)

---
 modules/ui/intro/welcome.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/modules/ui/intro/welcome.js b/modules/ui/intro/welcome.js
index 8187998e7..81e1f9b09 100644
--- a/modules/ui/intro/welcome.js
+++ b/modules/ui/intro/welcome.js
@@ -137,7 +137,7 @@ export function uiIntroWelcome(context, reveal) {
         );
     }
 
-    
+
     chapter.enter = function() {
         welcome();
     };
@@ -145,7 +145,7 @@ export function uiIntroWelcome(context, reveal) {
 
     chapter.exit = function() {
         listener.off();
-        var tooltip = d3_select('.curtain-tooltip.intro-mouse')
+        d3_select('.curtain-tooltip.intro-mouse')
             .selectAll('.counter')
             .remove();
     };

From 954227be53e467ea316ea9662941fb3ac18b8c28 Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Thu, 4 Jan 2018 16:12:40 -0500
Subject: [PATCH 02/10] Don't let modal button cover up header bottom border

---
 css/80_app.css | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/css/80_app.css b/css/80_app.css
index 2e14ac1d0..e10487025 100644
--- a/css/80_app.css
+++ b/css/80_app.css
@@ -713,7 +713,7 @@ button.save.has-count .count::before {
     position: absolute;
     right: 0;
     top: 0;
-    height: 60px;
+    height: 59px;
     z-index: 50;
 }
 [dir='rtl'] .modal > button {

From 8914d1ce367876863b11abd5fb94281411f267cc Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Thu, 4 Jan 2018 16:13:13 -0500
Subject: [PATCH 03/10] Variable cleanups and formatting

---
 modules/modes/save.js | 53 +++++++++++++++++++++----------------------
 modules/ui/commit.js  | 52 +++++++++++++++++++++---------------------
 modules/ui/loading.js | 25 ++++++++++----------
 3 files changed, 65 insertions(+), 65 deletions(-)

diff --git a/modules/modes/save.js b/modules/modes/save.js
index 4fb3908a0..212fca0d1 100644
--- a/modules/modes/save.js
+++ b/modules/modes/save.js
@@ -67,18 +67,17 @@ export function modeSave(context) {
 
 
     function save(changeset, tryAgain) {
-
-        var osm = context.connection(),
-            loading = uiLoading(context).message(t('save.uploading')).blocking(true),
-            history = context.history(),
-            origChanges = history.changes(actionDiscardTags(history.difference())),
-            localGraph = context.graph(),
-            remoteGraph = coreGraph(history.base(), true),
-            modified = _filter(history.difference().summary(), {changeType: 'modified'}),
-            toCheck = _map(_map(modified, 'entity'), 'id'),
-            toLoad = withChildNodes(toCheck, localGraph),
-            conflicts = [],
-            errors = [];
+        var osm = context.connection();
+        var loading = uiLoading(context).message(t('save.uploading')).blocking(true);
+        var history = context.history();
+        var origChanges = history.changes(actionDiscardTags(history.difference()));
+        var localGraph = context.graph();
+        var remoteGraph = coreGraph(history.base(), true);
+        var modified = _filter(history.difference().summary(), {changeType: 'modified'});
+        var toCheck = _map(_map(modified, 'entity'), 'id');
+        var toLoad = withChildNodes(toCheck, localGraph);
+        var conflicts = [];
+        var errors = [];
 
         if (!osm) return;
 
@@ -100,8 +99,8 @@ export function modeSave(context) {
                 var entity = graph.entity(id);
                 if (entity.type === 'way') {
                     try {
-                        var cn = graph.childNodes(entity);
-                        result.push.apply(result, _map(_filter(cn, 'version'), 'id'));
+                        var children = graph.childNodes(entity);
+                        result.push.apply(result, _map(_filter(children, 'version'), 'id'));
                     } catch (err) {
                         /* eslint-disable no-console */
                         if (typeof console !== 'undefined') console.error(err);
@@ -172,8 +171,8 @@ export function modeSave(context) {
                     var children = _union(local.nodes, remote.nodes);
 
                     for (var i = 0; i < children.length; i++) {
-                        var a = localGraph.hasEntity(children[i]),
-                            b = remoteGraph.hasEntity(children[i]);
+                        var a = localGraph.hasEntity(children[i]);
+                        var b = remoteGraph.hasEntity(children[i]);
 
                         if (a && b && a.version !== b.version) return false;
                     }
@@ -183,23 +182,23 @@ export function modeSave(context) {
             }
 
             _each(toCheck, function(id) {
-                var local = localGraph.entity(id),
-                    remote = remoteGraph.entity(id);
+                var local = localGraph.entity(id);
+                var remote = remoteGraph.entity(id);
 
                 if (compareVersions(local, remote)) return;
 
-                var action = actionMergeRemoteChanges,
-                    merge = action(id, localGraph, remoteGraph, formatUser);
+                var action = actionMergeRemoteChanges;
+                var merge = action(id, localGraph, remoteGraph, formatUser);
 
                 history.replace(merge);
 
                 var mergeConflicts = merge.conflicts();
                 if (!mergeConflicts.length) return;  // merged safely
 
-                var forceLocal = action(id, localGraph, remoteGraph).withOption('force_local'),
-                    forceRemote = action(id, localGraph, remoteGraph).withOption('force_remote'),
-                    keepMine = t('save.conflict.' + (remote.visible ? 'keep_local' : 'restore')),
-                    keepTheirs = t('save.conflict.' + (remote.visible ? 'keep_remote' : 'delete'));
+                var forceLocal = action(id, localGraph, remoteGraph).withOption('force_local');
+                var forceRemote = action(id, localGraph, remoteGraph).withOption('force_remote');
+                var keepMine = t('save.conflict.' + (remote.visible ? 'keep_local' : 'restore'));
+                var keepTheirs = t('save.conflict.' + (remote.visible ? 'keep_remote' : 'delete'));
 
                 conflicts.push({
                     id: id,
@@ -328,9 +327,9 @@ export function modeSave(context) {
                 .classed('hide-toggle', true)
                 .text(function(d) { return d.msg || t('save.unknown_error_details'); })
                 .on('click', function() {
-                    var error = d3_select(this),
-                        detail = d3_select(this.nextElementSibling),
-                        exp = error.classed('expanded');
+                    var error = d3_select(this);
+                    var detail = d3_select(this.nextElementSibling);
+                    var exp = error.classed('expanded');
 
                     detail.style('display', exp ? 'none' : 'block');
                     error.classed('expanded', !exp);
diff --git a/modules/ui/commit.js b/modules/ui/commit.js
index e7d3837f5..d4af06590 100644
--- a/modules/ui/commit.js
+++ b/modules/ui/commit.js
@@ -16,9 +16,9 @@ import { utilDetect } from '../util/detect';
 import { utilRebind } from '../util';
 
 
-var changeset;
+var _changeset;
 var readOnlyTags = [
-    /^changesets_count$/,
+    /^_changesets_count$/,
     /^created_by$/,
     /^ideditor:/,
     /^imagery_used$/,
@@ -32,9 +32,9 @@ var hashtagRegex = /(#[^\u2000-\u206F\u2E00-\u2E7F\s\\'!"#$%()*,.\/:;<=>?@\[\]^`
 
 
 export function uiCommit(context) {
-    var dispatch = d3_dispatch('cancel', 'save'),
-        userDetails,
-        _selection;
+    var dispatch = d3_dispatch('cancel', 'save');
+    var _userDetails;
+    var _selection;
 
     var changesetEditor = uiChangesetEditor(context)
         .on('change', changeTags);
@@ -51,16 +51,16 @@ export function uiCommit(context) {
         if (!osm) return;
 
         // expire stored comment and hashtags after cutoff datetime - #3947
-        var commentDate = +context.storage('commentDate') || 0,
-            currDate = Date.now(),
-            cutoff = 2 * 86400 * 1000;   // 2 days
+        var commentDate = +context.storage('commentDate') || 0;
+        var currDate = Date.now();
+        var cutoff = 2 * 86400 * 1000;   // 2 days
         if (commentDate > currDate || currDate - commentDate > cutoff) {
             context.storage('comment', null);
             context.storage('hashtags', null);
         }
 
         var tags;
-        if (!changeset) {
+        if (!_changeset) {
             var detected = utilDetect();
             tags = {
                 comment: context.storage('comment') || '',
@@ -78,12 +78,12 @@ export function uiCommit(context) {
                 tags.hashtags = hashtags;
             }
 
-            changeset = new osmChangeset({ tags: tags });
+            _changeset = new osmChangeset({ tags: tags });
         }
 
-        tags = _clone(changeset.tags);
+        tags = _clone(_changeset.tags);
         tags.imagery_used = context.history().imageryUsed().join(';').substr(0, 255);
-        changeset = changeset.update({ tags: tags });
+        _changeset = _changeset.update({ tags: tags });
 
         var header = selection.selectAll('.header')
             .data([0]);
@@ -114,7 +114,7 @@ export function uiCommit(context) {
 
         changesetSection
             .call(changesetEditor
-                .changesetID(changeset.id)
+                .changesetID(_changeset.id)
                 .tags(tags)
             );
 
@@ -146,7 +146,7 @@ export function uiCommit(context) {
 
             var userLink = d3_select(document.createElement('div'));
 
-            userDetails = user;
+            _userDetails = user;
 
             if (user.image_url) {
                 userLink
@@ -195,7 +195,7 @@ export function uiCommit(context) {
             .merge(requestReviewEnter);
 
         var requestReviewInput = requestReview.selectAll('input')
-            .property('checked', isReviewRequested(changeset.tags))
+            .property('checked', isReviewRequested(_changeset.tags))
             .on('change', toggleRequestReview);
 
 
@@ -238,7 +238,7 @@ export function uiCommit(context) {
                 return (n && n.value.length) ? null : true;
             })
             .on('click.save', function() {
-                dispatch.call('save', this, changeset);
+                dispatch.call('save', this, _changeset);
             });
 
 
@@ -256,7 +256,7 @@ export function uiCommit(context) {
             .call(rawTagEditor
                 .expanded(expanded)
                 .readOnlyTags(readOnlyTags)
-                .tags(_clone(changeset.tags))
+                .tags(_clone(_changeset.tags))
             );
 
 
@@ -273,7 +273,7 @@ export function uiCommit(context) {
                 .call(rawTagEditor
                     .expanded(expanded)
                     .readOnlyTags(readOnlyTags)
-                    .tags(_clone(changeset.tags))
+                    .tags(_clone(_changeset.tags))
                 );
         }
     }
@@ -299,8 +299,8 @@ export function uiCommit(context) {
 
 
     function findHashtags(tags, commentOnly) {
-        var inComment = commentTags(),
-            inHashTags = hashTags();
+        var inComment = commentTags();
+        var inHashTags = hashTags();
 
         if (inComment !== null) {                    // when hashtags are detected in comment...
             context.storage('hashtags', null);       // always remove stored hashtags - #4304
@@ -340,7 +340,7 @@ export function uiCommit(context) {
 
 
     function updateChangeset(changed, onInput) {
-        var tags = _clone(changeset.tags);
+        var tags = _clone(_changeset.tags);
 
         _forEach(changed, function(v, k) {
             k = k.trim().substr(0, 255);
@@ -371,8 +371,8 @@ export function uiCommit(context) {
         }
 
         // always update userdetails, just in case user reauthenticates as someone else
-        if (userDetails && userDetails.changesets_count !== undefined) {
-            var changesetsCount = parseInt(userDetails.changesets_count, 10) + 1;  // #4283
+        if (_userDetails && _userDetails.changesets_count !== undefined) {
+            var changesetsCount = parseInt(_userDetails.changesets_count, 10) + 1;  // #4283
             tags.changesets_count = String(changesetsCount);
 
             // first 100 edits - new user
@@ -397,14 +397,14 @@ export function uiCommit(context) {
             delete tags.changesets_count;
         }
 
-        if (!_isEqual(changeset.tags, tags)) {
-            changeset = changeset.update({ tags: tags });
+        if (!_isEqual(_changeset.tags, tags)) {
+            _changeset = _changeset.update({ tags: tags });
         }
     }
 
 
     commit.reset = function() {
-        changeset = null;
+        _changeset = null;
     };
 
 
diff --git a/modules/ui/loading.js b/modules/ui/loading.js
index 0b567fc71..899c56ae6 100644
--- a/modules/ui/loading.js
+++ b/modules/ui/loading.js
@@ -1,16 +1,17 @@
+import { select as d3_select } from 'd3-selection';
 import { uiModal } from './modal';
 
 
 export function uiLoading(context) {
-    var message = '',
-        blocking = false,
-        modalSelection;
+    var _modalSelection = d3_select(null);
+    var _message = '';
+    var _blocking = false;
 
 
     var loading = function(selection) {
-        modalSelection = uiModal(selection, blocking);
+        _modalSelection = uiModal(selection, _blocking);
 
-        var loadertext = modalSelection.select('.content')
+        var loadertext = _modalSelection.select('.content')
             .classed('loading-modal', true)
             .append('div')
             .attr('class', 'modal-section fillL');
@@ -22,9 +23,9 @@ export function uiLoading(context) {
 
         loadertext
             .append('h3')
-            .text(message);
+            .text(_message);
 
-        modalSelection.select('button.close')
+        _modalSelection.select('button.close')
             .attr('class', 'hide');
 
         return loading;
@@ -32,21 +33,21 @@ export function uiLoading(context) {
 
 
     loading.message = function(_) {
-        if (!arguments.length) return message;
-        message = _;
+        if (!arguments.length) return _message;
+        _message = _;
         return loading;
     };
 
 
     loading.blocking = function(_) {
-        if (!arguments.length) return blocking;
-        blocking = _;
+        if (!arguments.length) return _blocking;
+        _blocking = _;
         return loading;
     };
 
 
     loading.close = function() {
-        modalSelection.remove();
+        _modalSelection.remove();
     };
 
 

From 437893ebb8b31e033e6544f0cc343725c4d6a0fd Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Thu, 4 Jan 2018 23:27:00 -0500
Subject: [PATCH 04/10] Don't reenter putChangeset, allow reuse of open
 changeset id

We are going to start trying an opportunistic save, then only
start the conflict resolution stuff if the server returns a 409.
Reusing an already open changeset makes sense in this situation.
---
 modules/services/osm.js | 230 +++++++++++++++++++++-------------------
 1 file changed, 123 insertions(+), 107 deletions(-)

diff --git a/modules/services/osm.js b/modules/services/osm.js
index af96685a3..d284d72f2 100644
--- a/modules/services/osm.js
+++ b/modules/services/osm.js
@@ -25,25 +25,26 @@ import {
 import { utilRebind, utilIdleWorker } from '../util';
 
 
-var dispatch = d3_dispatch('authLoading', 'authDone', 'change', 'loading', 'loaded'),
-    urlroot = 'https://www.openstreetmap.org',
-    blacklists = ['.*\.google(apis)?\..*/(vt|kh)[\?/].*([xyz]=.*){3}.*'],
-    inflight = {},
-    loadedTiles = {},
-    entityCache = {},
-    connectionId = 1,
-    tileZoom = 16,
-    oauth = osmAuth({
-        url: urlroot,
-        oauth_consumer_key: '5A043yRSEugj4DJ5TljuapfnrflWDte8jTOcWLlT',
-        oauth_secret: 'aB3jKq1TRsCOUrfOIZ6oQMEDmv2ptV76PA54NGLL',
-        loading: authLoading,
-        done: authDone
-    }),
-    rateLimitError,
-    userChangesets,
-    userDetails,
-    off;
+var dispatch = d3_dispatch('authLoading', 'authDone', 'change', 'loading', 'loaded');
+var urlroot = 'https://www.openstreetmap.org';
+var oauth = osmAuth({
+    url: urlroot,
+    oauth_consumer_key: '5A043yRSEugj4DJ5TljuapfnrflWDte8jTOcWLlT',
+    oauth_secret: 'aB3jKq1TRsCOUrfOIZ6oQMEDmv2ptV76PA54NGLL',
+    loading: authLoading,
+    done: authDone
+});
+
+var _blacklists = ['.*\.google(apis)?\..*/(vt|kh)[\?/].*([xyz]=.*){3}.*'];
+var _tiles = { loaded: {}, inflight: {} };
+var _changeset = {};
+var _entityCache = {};
+var _connectionID = 1;
+var _tileZoom = 16;
+var _rateLimitError;
+var _userChangesets;
+var _userDetails;
+var _off;
 
 
 function authLoading() {
@@ -64,15 +65,15 @@ function abortRequest(i) {
 
 
 function getLoc(attrs) {
-    var lon = attrs.lon && attrs.lon.value,
-        lat = attrs.lat && attrs.lat.value;
+    var lon = attrs.lon && attrs.lon.value;
+    var lat = attrs.lat && attrs.lat.value;
     return [parseFloat(lon), parseFloat(lat)];
 }
 
 
 function getNodes(obj) {
-    var elems = obj.getElementsByTagName('nd'),
-        nodes = new Array(elems.length);
+    var elems = obj.getElementsByTagName('nd');
+    var nodes = new Array(elems.length);
     for (var i = 0, l = elems.length; i < l; i++) {
         nodes[i] = 'n' + elems[i].attributes.ref.value;
     }
@@ -81,8 +82,8 @@ function getNodes(obj) {
 
 
 function getTags(obj) {
-    var elems = obj.getElementsByTagName('tag'),
-        tags = {};
+    var elems = obj.getElementsByTagName('tag');
+    var tags = {};
     for (var i = 0, l = elems.length; i < l; i++) {
         var attrs = elems[i].attributes;
         tags[attrs.k.value] = attrs.v.value;
@@ -93,8 +94,8 @@ function getTags(obj) {
 
 
 function getMembers(obj) {
-    var elems = obj.getElementsByTagName('member'),
-        members = new Array(elems.length);
+    var elems = obj.getElementsByTagName('member');
+    var members = new Array(elems.length);
     for (var i = 0, l = elems.length; i < l; i++) {
         var attrs = elems[i].attributes;
         members[i] = {
@@ -164,14 +165,14 @@ function parse(xml, callback, options) {
     options = _extend({ cache: true }, options);
     if (!xml || !xml.childNodes) return;
 
-    var root = xml.childNodes[0],
-        children = root.childNodes;
+    var root = xml.childNodes[0];
+    var children = root.childNodes;
 
     function parseChild(child) {
         var parser = parsers[child.nodeName];
         if (parser) {
             var uid = osmEntity.id.fromOSM(child.nodeName, child.attributes.id.value);
-            if (options.cache && entityCache[uid]) {
+            if (options.cache && _entityCache[uid]) {
                 return null;
             }
             return parser(child, uid);
@@ -190,20 +191,21 @@ export default {
 
 
     reset: function() {
-        connectionId++;
-        userChangesets = undefined;
-        userDetails = undefined;
-        rateLimitError = undefined;
-        _forEach(inflight, abortRequest);
-        entityCache = {};
-        loadedTiles = {};
-        inflight = {};
+        _connectionID++;
+        _userChangesets = undefined;
+        _userDetails = undefined;
+        _rateLimitError = undefined;
+        _forEach(_tiles.inflight, abortRequest);
+        if (_changeset.inflight) abortRequest(_changeset.inflight);
+        _tiles = { loaded: {}, inflight: {} };
+        _changeset = {};
+        _entityCache = {};
         return this;
     },
 
 
     getConnectionId: function() {
-        return connectionId;
+        return _connectionID;
     },
 
 
@@ -239,7 +241,7 @@ export default {
     loadFromAPI: function(path, callback, options) {
         options = _extend({ cache: true }, options);
         var that = this;
-        var cid = connectionId;
+        var cid = _connectionID;
 
         function done(err, xml) {
             if (that.getConnectionId() !== cid) {
@@ -260,9 +262,9 @@ export default {
             } else {
                 // 509 Bandwidth Limit Exceeded, 429 Too Many Requests
                 // Set the rateLimitError flag and trigger a warning..
-                if (!isAuthenticated && !rateLimitError && err &&
+                if (!isAuthenticated && !_rateLimitError && err &&
                         (err.status === 509 || err.status === 429)) {
-                    rateLimitError = err;
+                    _rateLimitError = err;
                     dispatch.call('change');
                 }
 
@@ -271,7 +273,7 @@ export default {
                     parse(xml, function (entities) {
                         if (options.cache) {
                             for (var i in entities) {
-                                entityCache[entities[i].id] = true;
+                                _entityCache[entities[i].id] = true;
                             }
                         }
                         callback(null, entities);
@@ -290,9 +292,9 @@ export default {
 
 
     loadEntity: function(id, callback) {
-        var type = osmEntity.id.type(id),
-            osmID = osmEntity.id.toOSM(id),
-            options = { cache: false };
+        var type = osmEntity.id.type(id);
+        var osmID = osmEntity.id.toOSM(id);
+        var options = { cache: false };
 
         this.loadFromAPI(
             '/api/0.6/' + type + '/' + osmID + (type !== 'node' ? '/full' : ''),
@@ -305,9 +307,9 @@ export default {
 
 
     loadEntityVersion: function(id, version, callback) {
-        var type = osmEntity.id.type(id),
-            osmID = osmEntity.id.toOSM(id),
-            options = { cache: false };
+        var type = osmEntity.id.type(id);
+        var osmID = osmEntity.id.toOSM(id);
+        var options = { cache: false };
 
         this.loadFromAPI(
             '/api/0.6/' + type + '/' + osmID + '/' + version,
@@ -323,9 +325,9 @@ export default {
         var that = this;
 
         _forEach(_groupBy(_uniq(ids), osmEntity.id.type), function(v, k) {
-            var type = k + 's',
-                osmIDs = _map(v, osmEntity.id.toOSM),
-                options = { cache: false };
+            var type = k + 's';
+            var osmIDs = _map(v, osmEntity.id.toOSM);
+            var options = { cache: false };
 
             _forEach(_chunk(osmIDs, 150), function(arr) {
                 that.loadFromAPI(
@@ -346,19 +348,28 @@ export default {
 
 
     putChangeset: function(changeset, changes, callback) {
+        if (_changeset.inflight) {
+            return callback({ message: 'Changeset already inflight', status: -2 });
+        }
+
         var that = this;
-        var cid = connectionId;
+        var cid = _connectionID;
 
-        // Create the changeset..
-        oauth.xhr({
-            method: 'PUT',
-            path: '/api/0.6/changeset/create',
-            options: { header: { 'Content-Type': 'text/xml' } },
-            content: JXON.stringify(changeset.asJXON())
-        }, createdChangeset);
+        if (_changeset.open) {   // reuse existing open changeset..
+            createdChangeset(null, _changeset.open);
+        } else {                 // open a new changeset..
+            _changeset.inflight = oauth.xhr({
+                method: 'PUT',
+                path: '/api/0.6/changeset/create',
+                options: { header: { 'Content-Type': 'text/xml' } },
+                content: JXON.stringify(changeset.asJXON())
+            }, createdChangeset);
+        }
 
 
-        function createdChangeset(err, changeset_id) {
+        function createdChangeset(err, changesetID) {
+            _changeset.inflight = null;
+
             if (err) {
                 return callback(err);
             }
@@ -366,12 +377,13 @@ export default {
                 return callback({ message: 'Connection Switched', status: -1 });
             }
 
-            changeset = changeset.update({ id: changeset_id });
+            _changeset.open = changesetID;
+            changeset = changeset.update({ id: changesetID });
 
             // Upload the changeset..
-            oauth.xhr({
+            _changeset.inflight = oauth.xhr({
                 method: 'POST',
-                path: '/api/0.6/changeset/' + changeset_id + '/upload',
+                path: '/api/0.6/changeset/' + changesetID + '/upload',
                 options: { header: { 'Content-Type': 'text/xml' } },
                 content: JXON.stringify(changeset.osmChangeJXON(changes))
             }, uploadedChangeset);
@@ -379,6 +391,8 @@ export default {
 
 
         function uploadedChangeset(err) {
+            _changeset.inflight = null;
+
             if (err) return callback(err);
 
             // Upload was successful, safe to call the callback.
@@ -387,6 +401,8 @@ export default {
                 callback(null, changeset);
             }, 2500);
 
+            _changeset.open = null;
+
             // At this point, we don't really care if the connection was switched..
             // Only try to close the changeset if we're still talking to the same server.
             if (that.getConnectionId() === cid) {
@@ -402,13 +418,13 @@ export default {
 
 
     userDetails: function(callback) {
-        if (userDetails) {
-            callback(undefined, userDetails);
+        if (_userDetails) {
+            callback(undefined, _userDetails);
             return;
         }
 
         var that = this;
-        var cid = connectionId;
+        var cid = _connectionID;
 
         function done(err, user_details) {
             if (err) {
@@ -418,29 +434,29 @@ export default {
                 return callback({ message: 'Connection Switched', status: -1 });
             }
 
-            var u = user_details.getElementsByTagName('user')[0],
-                img = u.getElementsByTagName('img'),
-                image_url = '';
+            var u = user_details.getElementsByTagName('user')[0];
+            var img = u.getElementsByTagName('img');
+            var image_url = '';
 
             if (img && img[0] && img[0].getAttribute('href')) {
                 image_url = img[0].getAttribute('href');
             }
 
-            var changesets = u.getElementsByTagName('changesets'),
-                changesets_count = 0;
+            var changesets = u.getElementsByTagName('changesets');
+            var changesets_count = 0;
 
             if (changesets && changesets[0] && changesets[0].getAttribute('count')) {
                 changesets_count = changesets[0].getAttribute('count');
             }
 
-            userDetails = {
+            _userDetails = {
                 id: u.attributes.id.value,
                 display_name: u.attributes.display_name.value,
                 image_url: image_url,
                 changesets_count: changesets_count
             };
 
-            callback(undefined, userDetails);
+            callback(undefined, _userDetails);
         }
 
         oauth.xhr({ method: 'GET', path: '/api/0.6/user/details' }, done);
@@ -448,13 +464,13 @@ export default {
 
 
     userChangesets: function(callback) {
-        if (userChangesets) {
-            callback(undefined, userChangesets);
+        if (_userChangesets) {
+            callback(undefined, _userChangesets);
             return;
         }
 
         var that = this;
-        var cid = connectionId;
+        var cid = _connectionID;
 
         this.userDetails(function(err, user) {
             if (err) {
@@ -472,7 +488,7 @@ export default {
                     return callback({ message: 'Connection Switched', status: -1 });
                 }
 
-                userChangesets = Array.prototype.map.call(
+                _userChangesets = Array.prototype.map.call(
                     changesets.getElementsByTagName('changeset'),
                     function (changeset) {
                         return { tags: getTags(changeset) };
@@ -482,7 +498,7 @@ export default {
                     return comment && comment !== '';
                 });
 
-                callback(undefined, userChangesets);
+                callback(undefined, _userChangesets);
             }
 
             oauth.xhr({ method: 'GET', path: '/api/0.6/changesets?user=' + user.id }, done);
@@ -492,7 +508,7 @@ export default {
 
     status: function(callback) {
         var that = this;
-        var cid = connectionId;
+        var cid = _connectionID;
 
         function done(xml) {
             if (that.getConnectionId() !== cid) {
@@ -509,12 +525,12 @@ export default {
                 }
             }
             if (regexes.length) {
-                blacklists = regexes;
+                _blacklists = regexes;
             }
 
 
-            if (rateLimitError) {
-                callback(rateLimitError, 'rateLimited');
+            if (_rateLimitError) {
+                callback(_rateLimitError, 'rateLimited');
             } else {
                 var apiStatus = xml.getElementsByTagName('status');
                 var val = apiStatus[0].getAttribute('api');
@@ -530,31 +546,31 @@ export default {
 
 
     imageryBlacklists: function() {
-        return blacklists;
+        return _blacklists;
     },
 
 
     tileZoom: function(_) {
-        if (!arguments.length) return tileZoom;
-        tileZoom = _;
+        if (!arguments.length) return _tileZoom;
+        _tileZoom = _;
         return this;
     },
 
 
     loadTiles: function(projection, dimensions, callback) {
-        if (off) return;
+        if (_off) return;
 
         var that = this;
         var s = projection.scale() * 2 * Math.PI;
         var z = Math.max(Math.log(s) / Math.log(2) - 8, 0);
-        var ts = 256 * Math.pow(2, z - tileZoom);
+        var ts = 256 * Math.pow(2, z - _tileZoom);
         var origin = [
             s / 2 - projection.translate()[0],
             s / 2 - projection.translate()[1]
         ];
 
         var tiles = d3_geoTile()
-            .scaleExtent([tileZoom, tileZoom])
+            .scaleExtent([_tileZoom, _tileZoom])
             .scale(s)
             .size(dimensions)
             .translate(projection.translate())()
@@ -570,36 +586,36 @@ export default {
                 };
             });
 
-        _filter(inflight, function(v, i) {
+        _filter(_tiles.inflight, function(v, i) {
             var wanted = _find(tiles, function(tile) {
                 return i === tile.id;
             });
-            if (!wanted) delete inflight[i];
+            if (!wanted) delete _tiles.inflight[i];
             return !wanted;
         }).map(abortRequest);
 
         tiles.forEach(function(tile) {
             var id = tile.id;
 
-            if (loadedTiles[id] || inflight[id]) return;
+            if (_tiles.loaded[id] || _tiles.inflight[id]) return;
 
-            if (_isEmpty(inflight)) {
+            if (_isEmpty(_tiles.inflight)) {
                 dispatch.call('loading');
             }
 
-            inflight[id] = that.loadFromAPI(
+            _tiles.inflight[id] = that.loadFromAPI(
                 '/api/0.6/map?bbox=' + tile.extent.toParam(),
                 function(err, parsed) {
-                    delete inflight[id];
+                    delete _tiles.inflight[id];
                     if (!err) {
-                        loadedTiles[id] = true;
+                        _tiles.loaded[id] = true;
                     }
 
                     if (callback) {
                         callback(err, _extend({ data: parsed }, tile));
                     }
 
-                    if (_isEmpty(inflight)) {
+                    if (_isEmpty(_tiles.inflight)) {
                         dispatch.call('loaded');
                     }
                 }
@@ -625,21 +641,21 @@ export default {
 
 
     toggle: function(_) {
-        off = !_;
+        _off = !_;
         return this;
     },
 
 
     loadedTiles: function(_) {
-        if (!arguments.length) return loadedTiles;
-        loadedTiles = _;
+        if (!arguments.length) return _tiles.loaded;
+        _tiles.loaded = _;
         return this;
     },
 
 
     logout: function() {
-        userChangesets = undefined;
-        userDetails = undefined;
+        _userChangesets = undefined;
+        _userDetails = undefined;
         oauth.logout();
         dispatch.call('change');
         return this;
@@ -648,9 +664,9 @@ export default {
 
     authenticate: function(callback) {
         var that = this;
-        var cid = connectionId;
-        userChangesets = undefined;
-        userDetails = undefined;
+        var cid = _connectionID;
+        _userChangesets = undefined;
+        _userDetails = undefined;
 
         function done(err, res) {
             if (err) {
@@ -661,10 +677,10 @@ export default {
                 if (callback) callback({ message: 'Connection Switched', status: -1 });
                 return;
             }
-            rateLimitError = undefined;
+            _rateLimitError = undefined;
             dispatch.call('change');
             if (callback) callback(err, res);
-            that.userChangesets(function() {});  // eagerly load user details/changesets
+            that._userChangesets(function() {});  // eagerly load user details/changesets
         }
 
         return oauth.authenticate(done);

From a63c4a72fe3b215b0c78c92e1ccecfd871e7e598 Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Fri, 5 Jan 2018 14:40:59 -0500
Subject: [PATCH 05/10] Improvements to save flow - Attempt fast save first,
 only perform conflict resolution if necessary (re: #3056) - Block reentry of
 save, and dont keep focus on save button (closes #4641) - Refactor modeSave()
 for code clarity (avoid shared state in closure variables)

---
 modules/modes/save.js   | 351 ++++++++++++++++++++++------------------
 modules/services/osm.js |  10 +-
 modules/ui/commit.js    |   1 +
 modules/ui/confirm.js   |   4 +-
 modules/ui/conflicts.js |  51 +++---
 5 files changed, 230 insertions(+), 187 deletions(-)

diff --git a/modules/modes/save.js b/modules/modes/save.js
index 212fca0d1..73479e72a 100644
--- a/modules/modes/save.js
+++ b/modules/modes/save.js
@@ -1,7 +1,6 @@
 import _clone from 'lodash-es/clone';
 import _difference from 'lodash-es/difference';
 import _filter from 'lodash-es/filter';
-import _each from 'lodash-es/each';
 import _map from 'lodash-es/map';
 import _reduce from 'lodash-es/reduce';
 import _union from 'lodash-es/union';
@@ -44,18 +43,27 @@ import {
 } from '../util';
 
 
+var _isSaving = false;
+
 
 export function modeSave(context) {
-    var mode = {
-        id: 'save'
-    };
-
+    var mode = { id: 'save' };
     var keybinding = d3_keybinding('select');
 
+    var loading = uiLoading(context)
+        .message(t('save.uploading'))
+        .blocking(true);
+
     var commit = uiCommit(context)
         .on('cancel', cancel)
         .on('save', save);
 
+    var _toCheck = [];
+    var _toLoad = [];
+    var _conflicts = [];
+    var _errors = [];
+    var _origChanges;
+
 
     function cancel(selectedID) {
         if (selectedID) {
@@ -66,33 +74,50 @@ export function modeSave(context) {
     }
 
 
-    function save(changeset, tryAgain) {
-        var osm = context.connection();
-        var loading = uiLoading(context).message(t('save.uploading')).blocking(true);
-        var history = context.history();
-        var origChanges = history.changes(actionDiscardTags(history.difference()));
-        var localGraph = context.graph();
-        var remoteGraph = coreGraph(history.base(), true);
-        var modified = _filter(history.difference().summary(), {changeType: 'modified'});
-        var toCheck = _map(_map(modified, 'entity'), 'id');
-        var toLoad = withChildNodes(toCheck, localGraph);
-        var conflicts = [];
-        var errors = [];
+    function save(changeset, tryAgain, checkConflicts) {
+        // Guard against accidentally entering save code twice - #4641
+        if (_isSaving && !tryAgain) return;
 
+        var osm = context.connection();
         if (!osm) return;
 
+        _isSaving = true;
+        context.container().call(loading);  // block input
+
+        var history = context.history();
+        var localGraph = context.graph();
+        var remoteGraph = coreGraph(history.base(), true);
+
+        _conflicts = [];
+        _errors = [];
+
+        // Store original changes, in case user wants to download them as an .osc file
+        _origChanges = history.changes(actionDiscardTags(history.difference()));
+
+        // First time, `history.perform` a no-op action.
+        // Any conflict resolutions will be done as `history.replace`
         if (!tryAgain) {
-            history.perform(actionNoop());  // checkpoint
+            history.perform(actionNoop());
         }
 
-        context.container().call(loading);
+        // Attempt a fast upload.. If there are conflicts, re-enter with `checkConflicts = true`
+        if (!checkConflicts) {
+            upload(changeset);
 
-        if (toCheck.length) {
-            osm.loadMultiple(toLoad, loaded);
+        // Do the full (slow) conflict check..
         } else {
-            upload();
+            var modified = _filter(history.difference().summary(), { changeType: 'modified' });
+            _toCheck = _map(_map(modified, 'entity'), 'id');
+            _toLoad = withChildNodes(_toCheck, localGraph);
+            if (_toCheck.length) {
+                osm.loadMultiple(_toLoad, loaded);
+            } else {
+                upload(changeset);
+            }
         }
 
+        return;
+
 
         function withChildNodes(ids, graph) {
             return _uniq(_reduce(ids, function(result, id) {
@@ -111,13 +136,12 @@ export function modeSave(context) {
             }, _clone(ids)));
         }
 
-
         // Reload modified entities into an alternate graph and check for conflicts..
         function loaded(err, result) {
-            if (errors.length) return;
+            if (_errors.length) return;
 
             if (err) {
-                errors.push({
+                _errors.push({
                     msg: err.responseText,
                     details: [ t('save.status_code', { code: err.status }) ]
                 });
@@ -125,35 +149,35 @@ export function modeSave(context) {
 
             } else {
                 var loadMore = [];
-                _each(result.data, function(entity) {
+                result.data.forEach(function(entity) {
                     remoteGraph.replace(entity);
-                    toLoad = _without(toLoad, entity.id);
+                    _toLoad = _without(_toLoad, entity.id);
 
                     // Because loadMultiple doesn't download /full like loadEntity,
                     // need to also load children that aren't already being checked..
                     if (!entity.visible) return;
                     if (entity.type === 'way') {
                         loadMore.push.apply(loadMore,
-                            _difference(entity.nodes, toCheck, toLoad, loadMore));
+                            _difference(entity.nodes, _toCheck, _toLoad, loadMore));
                     } else if (entity.type === 'relation' && entity.isMultipolygon()) {
                         loadMore.push.apply(loadMore,
-                            _difference(_map(entity.members, 'id'), toCheck, toLoad, loadMore));
+                            _difference(_map(entity.members, 'id'), _toCheck, _toLoad, loadMore));
                     }
                 });
 
                 if (loadMore.length) {
-                    toLoad.push.apply(toLoad, loadMore);
+                    _toLoad.push.apply(_toLoad, loadMore);
                     osm.loadMultiple(loadMore, loaded);
                 }
 
-                if (!toLoad.length) {
-                    checkConflicts();
+                if (!_toLoad.length) {
+                    detectConflicts();
                 }
             }
         }
 
 
-        function checkConflicts() {
+        function detectConflicts() {
             function choice(id, text, action) {
                 return { id: id, text: text, action: function() { history.replace(action); } };
             }
@@ -164,16 +188,14 @@ export function modeSave(context) {
                 return utilDisplayName(entity) || (utilDisplayType(entity.id) + ' ' + entity.id);
             }
 
-            function compareVersions(local, remote) {
+            function sameVersions(local, remote) {
                 if (local.version !== remote.version) return false;
 
                 if (local.type === 'way') {
                     var children = _union(local.nodes, remote.nodes);
-
                     for (var i = 0; i < children.length; i++) {
                         var a = localGraph.hasEntity(children[i]);
                         var b = remoteGraph.hasEntity(children[i]);
-
                         if (a && b && a.version !== b.version) return false;
                     }
                 }
@@ -181,11 +203,11 @@ export function modeSave(context) {
                 return true;
             }
 
-            _each(toCheck, function(id) {
+            _toCheck.forEach(function(id) {
                 var local = localGraph.entity(id);
                 var remote = remoteGraph.entity(id);
 
-                if (compareVersions(local, remote)) return;
+                if (sameVersions(local, remote)) return;
 
                 var action = actionMergeRemoteChanges;
                 var merge = action(id, localGraph, remoteGraph, formatUser);
@@ -200,7 +222,7 @@ export function modeSave(context) {
                 var keepMine = t('save.conflict.' + (remote.visible ? 'keep_local' : 'restore'));
                 var keepTheirs = t('save.conflict.' + (remote.visible ? 'keep_remote' : 'delete'));
 
-                conflicts.push({
+                _conflicts.push({
                     id: id,
                     name: entityName(local),
                     details: mergeConflicts,
@@ -212,163 +234,179 @@ export function modeSave(context) {
                 });
             });
 
-            upload();
+            upload(changeset);
+        }
+    }
+
+
+    function upload(changeset) {
+        var osm = context.connection();
+        if (!osm) {
+            _errors.push({ msg: 'No OSM Service' });
         }
 
+        if (_conflicts.length) {
+            _conflicts.sort(function(a, b) { return b.id.localeCompare(a.id); });
+            showConflicts(changeset);
 
-        function upload() {
-            if (conflicts.length) {
-                conflicts.sort(function(a,b) { return b.id.localeCompare(a.id); });
-                showConflicts();
-            } else if (errors.length) {
-                showErrors();
-            } else {
-                var changes = history.changes(actionDiscardTags(history.difference()));
-                if (changes.modified.length || changes.created.length || changes.deleted.length) {
-                    osm.putChangeset(changeset, changes, uploadCallback);
-                } else {        // changes were insignificant or reverted by user
-                    d3_select('.inspector-wrap *').remove();
-                    loading.close();
-                    context.flush();
-                    cancel();
-                }
+        } else if (_errors.length) {
+            showErrors();
+
+        } else {
+            var history = context.history();
+            var changes = history.changes(actionDiscardTags(history.difference()));
+            if (changes.modified.length || changes.created.length || changes.deleted.length) {
+                osm.putChangeset(changeset, changes, uploadCallback);
+            } else {        // changes were insignificant or reverted by user
+                d3_select('.inspector-wrap *').remove();
+                loading.close();
+                _isSaving = false;
+                context.flush();
+                cancel();
             }
         }
+    }
 
 
-        function uploadCallback(err, changeset) {
-            if (err) {
-                errors.push({
+    function uploadCallback(err, changeset) {
+        if (err) {
+            if (err.status === 409) {          // 409 Conflict
+                save(changeset, true, true);   // tryAgain = true, checkConflicts = true
+            } else {
+                _errors.push({
                     msg: err.responseText,
                     details: [ t('save.status_code', { code: err.status }) ]
                 });
                 showErrors();
-            } else {
-                history.clearSaved();
-                success(changeset);
-                // Add delay to allow for postgres replication #1646 #2678
-                window.setTimeout(function() {
-                    d3_select('.inspector-wrap *').remove();
-                    loading.close();
-                    context.flush();
-                }, 2500);
             }
+
+        } else {
+            context.history().clearSaved();
+            success(changeset);
+            // Add delay to allow for postgres replication #1646 #2678
+            window.setTimeout(function() {
+                d3_select('.inspector-wrap *').remove();
+                loading.close();
+                _isSaving = false;
+                context.flush();
+            }, 2500);
         }
+    }
 
 
-        function showConflicts() {
-            var selection = context.container()
-                .select('#sidebar')
-                .append('div')
-                .attr('class','sidebar-component');
+    function showConflicts(changeset) {
+        var history = context.history();
+        var selection = context.container()
+            .select('#sidebar')
+            .append('div')
+            .attr('class','sidebar-component');
 
-            loading.close();
+        loading.close();
+        _isSaving = false;
 
-            selection.call(uiConflicts(context)
-                .list(conflicts)
-                .origChanges(origChanges)
-                .on('cancel', function() {
-                    history.pop();
-                    selection.remove();
-                })
-                .on('save', function() {
-                    for (var i = 0; i < conflicts.length; i++) {
-                        if (conflicts[i].chosen === 1) {  // user chose "keep theirs"
-                            var entity = context.hasEntity(conflicts[i].id);
-                            if (entity && entity.type === 'way') {
-                                var children = _uniq(entity.nodes);
-                                for (var j = 0; j < children.length; j++) {
-                                    history.replace(actionRevert(children[j]));
-                                }
+        var ui = uiConflicts(context)
+            .conflictList(_conflicts)
+            .origChanges(_origChanges)
+            .on('cancel', function() {
+                history.pop();
+                selection.remove();
+            })
+            .on('save', function() {
+                for (var i = 0; i < _conflicts.length; i++) {
+                    if (_conflicts[i].chosen === 1) {  // user chose "keep theirs"
+                        var entity = context.hasEntity(_conflicts[i].id);
+                        if (entity && entity.type === 'way') {
+                            var children = _uniq(entity.nodes);
+                            for (var j = 0; j < children.length; j++) {
+                                history.replace(actionRevert(children[j]));
                             }
-                            history.replace(actionRevert(conflicts[i].id));
                         }
+                        history.replace(actionRevert(_conflicts[i].id));
                     }
+                }
 
-                    selection.remove();
-                    save(changeset, true);
-                })
-            );
-        }
+                selection.remove();
+                save(changeset, true, false);  // tryAgain = true, checkConflicts = false
+            });
+
+        selection.call(ui);
+    }
 
 
-        function showErrors() {
-            var selection = uiConfirm(context.container());
+    function showErrors() {
+        var selection = uiConfirm(context.container());
 
-            history.pop();
-            loading.close();
+        context.history().pop();
+        loading.close();
+        _isSaving = false;
 
-            selection
-                .select('.modal-section.header')
-                .append('h3')
-                .text(t('save.error'));
+        selection
+            .select('.modal-section.header')
+            .append('h3')
+            .text(t('save.error'));
 
-            addErrors(selection, errors);
-            selection.okButton();
-        }
+        addErrors(selection, _errors);
+        selection.okButton();
+    }
 
 
-        function addErrors(selection, data) {
-            var message = selection
-                .select('.modal-section.message-text');
+    function addErrors(selection, data) {
+        var message = selection
+            .select('.modal-section.message-text');
 
-            var items = message
-                .selectAll('.error-container')
-                .data(data);
+        var items = message
+            .selectAll('.error-container')
+            .data(data);
 
-            var enter = items.enter()
-                .append('div')
-                .attr('class', 'error-container');
+        var enter = items.enter()
+            .append('div')
+            .attr('class', 'error-container');
 
-            enter
-                .append('a')
-                .attr('class', 'error-description')
-                .attr('href', '#')
-                .classed('hide-toggle', true)
-                .text(function(d) { return d.msg || t('save.unknown_error_details'); })
-                .on('click', function() {
-                    var error = d3_select(this);
-                    var detail = d3_select(this.nextElementSibling);
-                    var exp = error.classed('expanded');
+        enter
+            .append('a')
+            .attr('class', 'error-description')
+            .attr('href', '#')
+            .classed('hide-toggle', true)
+            .text(function(d) { return d.msg || t('save.unknown_error_details'); })
+            .on('click', function() {
+                d3_event.preventDefault();
 
-                    detail.style('display', exp ? 'none' : 'block');
-                    error.classed('expanded', !exp);
+                var error = d3_select(this);
+                var detail = d3_select(this.nextElementSibling);
+                var exp = error.classed('expanded');
 
-                    d3_event.preventDefault();
-                });
+                detail.style('display', exp ? 'none' : 'block');
+                error.classed('expanded', !exp);
+            });
 
-            var details = enter
-                .append('div')
-                .attr('class', 'error-detail-container')
-                .style('display', 'none');
+        var details = enter
+            .append('div')
+            .attr('class', 'error-detail-container')
+            .style('display', 'none');
 
-            details
-                .append('ul')
-                .attr('class', 'error-detail-list')
-                .selectAll('li')
-                .data(function(d) { return d.details || []; })
-                .enter()
-                .append('li')
-                .attr('class', 'error-detail-item')
-                .text(function(d) { return d; });
-
-            items.exit()
-                .remove();
-        }
+        details
+            .append('ul')
+            .attr('class', 'error-detail-list')
+            .selectAll('li')
+            .data(function(d) { return d.details || []; })
+            .enter()
+            .append('li')
+            .attr('class', 'error-detail-item')
+            .text(function(d) { return d; });
 
+        items.exit()
+            .remove();
     }
 
 
     function success(changeset) {
         commit.reset();
-        context.enter(modeBrowse(context)
-            .sidebar(uiSuccess(context)
-                .changeset(changeset)
-                .on('cancel', function() {
-                    context.ui().sidebar.hide();
-                })
-            )
-        );
+
+        var ui = uiSuccess(context)
+            .changeset(changeset)
+            .on('cancel', function() { context.ui().sidebar.hide(); });
+
+        context.enter(modeBrowse(context).sidebar(ui));
     }
 
 
@@ -404,6 +442,9 @@ export function modeSave(context) {
 
 
     mode.exit = function() {
+        loading.close();
+        _isSaving = false;
+
         keybinding.off();
 
         context.container().selectAll('#content')
diff --git a/modules/services/osm.js b/modules/services/osm.js
index d284d72f2..89b45013d 100644
--- a/modules/services/osm.js
+++ b/modules/services/osm.js
@@ -349,7 +349,7 @@ export default {
 
     putChangeset: function(changeset, changes, callback) {
         if (_changeset.inflight) {
-            return callback({ message: 'Changeset already inflight', status: -2 });
+            return callback({ message: 'Changeset already inflight', status: -2 }, changeset);
         }
 
         var that = this;
@@ -371,10 +371,10 @@ export default {
             _changeset.inflight = null;
 
             if (err) {
-                return callback(err);
+                return callback(err, changeset);
             }
             if (that.getConnectionId() !== cid) {
-                return callback({ message: 'Connection Switched', status: -1 });
+                return callback({ message: 'Connection Switched', status: -1 }, changeset);
             }
 
             _changeset.open = changesetID;
@@ -393,7 +393,7 @@ export default {
         function uploadedChangeset(err) {
             _changeset.inflight = null;
 
-            if (err) return callback(err);
+            if (err) return callback(err, changeset);
 
             // Upload was successful, safe to call the callback.
             // Add delay to allow for postgres replication #1646 #2678
@@ -680,7 +680,7 @@ export default {
             _rateLimitError = undefined;
             dispatch.call('change');
             if (callback) callback(err, res);
-            that._userChangesets(function() {});  // eagerly load user details/changesets
+            that.userChangesets(function() {});  // eagerly load user details/changesets
         }
 
         return oauth.authenticate(done);
diff --git a/modules/ui/commit.js b/modules/ui/commit.js
index d4af06590..be1ef0e72 100644
--- a/modules/ui/commit.js
+++ b/modules/ui/commit.js
@@ -238,6 +238,7 @@ export function uiCommit(context) {
                 return (n && n.value.length) ? null : true;
             })
             .on('click.save', function() {
+                this.blur();    // avoid keeping focus on the button - #4641
                 dispatch.call('save', this, _changeset);
             });
 
diff --git a/modules/ui/confirm.js b/modules/ui/confirm.js
index aa1238a20..2fd0e4383 100644
--- a/modules/ui/confirm.js
+++ b/modules/ui/confirm.js
@@ -27,7 +27,9 @@ export function uiConfirm(selection) {
             .on('click.confirm', function() {
                 modalSelection.remove();
             })
-            .text(t('confirm.okay'));
+            .text(t('confirm.okay'))
+            .node()
+            .focus();
 
         return modalSelection;
     };
diff --git a/modules/ui/conflicts.js b/modules/ui/conflicts.js
index 2cfe3448e..21a0aa2b7 100644
--- a/modules/ui/conflicts.js
+++ b/modules/ui/conflicts.js
@@ -16,9 +16,9 @@ import { utilRebind } from '../util/rebind';
 
 
 export function uiConflicts(context) {
-    var dispatch = d3_dispatch('cancel', 'save'),
-        origChanges,
-        conflictList;
+    var dispatch = d3_dispatch('cancel', 'save');
+    var _origChanges;
+    var _conflictList;
 
 
     function conflicts(selection) {
@@ -47,14 +47,14 @@ export function uiConflicts(context) {
 
 
         // Download changes link
-        var detected = utilDetect(),
-            changeset = new osmChangeset();
+        var detected = utilDetect();
+        var changeset = new osmChangeset();
 
-        delete changeset.id;  // Export without chnageset_id
+        delete changeset.id;  // Export without changeset_id
 
-        var data = JXON.stringify(changeset.osmChangeJXON(origChanges)),
-            blob = new Blob([data], {type: 'text/xml;charset=utf-8;'}),
-            fileName = 'changes.osc';
+        var data = JXON.stringify(changeset.osmChangeJXON(_origChanges));
+        var blob = new Blob([data], { type: 'text/xml;charset=utf-8;' });
+        var fileName = 'changes.osc';
 
         var linkEnter = conflictsHelp.selectAll('.download-changes')
             .data([0])
@@ -99,7 +99,7 @@ export function uiConflicts(context) {
 
         buttons
             .append('button')
-            .attr('disabled', conflictList.length > 1)
+            .attr('disabled', _conflictList.length > 1)
             .attr('class', 'action conflicts-button col6')
             .text(t('save.title'))
             .on('click.try_again', function() { dispatch.call('save'); });
@@ -113,12 +113,12 @@ export function uiConflicts(context) {
 
 
     function showConflict(selection, index) {
-        if (index < 0 || index >= conflictList.length) return;
+        if (index < 0 || index >= _conflictList.length) return;
 
         var parent = d3_select(selection.node().parentNode);
 
         // enable save button if this is the last conflict being reviewed..
-        if (index === conflictList.length - 1) {
+        if (index === _conflictList.length - 1) {
             window.setTimeout(function() {
                 parent.select('.conflicts-button')
                     .attr('disabled', null);
@@ -132,7 +132,7 @@ export function uiConflicts(context) {
 
         var item = selection
             .selectAll('.conflict')
-            .data([conflictList[index]]);
+            .data([_conflictList[index]]);
 
         var enter = item.enter()
             .append('div')
@@ -141,7 +141,7 @@ export function uiConflicts(context) {
         enter
             .append('h4')
             .attr('class', 'conflict-count')
-            .text(t('save.conflict.count', { num: index + 1, total: conflictList.length }));
+            .text(t('save.conflict.count', { num: index + 1, total: _conflictList.length }));
 
         enter
             .append('a')
@@ -183,11 +183,11 @@ export function uiConflicts(context) {
             .attr('class', 'conflict-nav-button action col6')
             .attr('disabled', function(d, i) {
                 return (i === 0 && index === 0) ||
-                    (i === 1 && index === conflictList.length - 1) || null;
+                    (i === 1 && index === _conflictList.length - 1) || null;
             })
             .on('click', function(d, i) {
-                var container = parent.select('.conflict-container'),
-                sign = (i === 0 ? -1 : 1);
+                var container = parent.select('.conflict-container');
+                var sign = (i === 0 ? -1 : 1);
 
                 container
                     .selectAll('.conflict')
@@ -249,8 +249,8 @@ export function uiConflicts(context) {
             .selectAll('input')
             .property('checked', function(d) { return d === datum; });
 
-        var extent = geoExtent(),
-            entity;
+        var extent = geoExtent();
+        var entity;
 
         entity = context.graph().hasEntity(datum.id);
         if (entity) extent._extend(entity.extent(context.graph()));
@@ -275,8 +275,7 @@ export function uiConflicts(context) {
             } else {
                 context.map().zoomTo(entity);
             }
-            context.surface().selectAll(
-                utilEntityOrMemberSelector([entity.id], context.graph()))
+            context.surface().selectAll(utilEntityOrMemberSelector([entity.id], context.graph()))
                 .classed('hover', true);
         }
     }
@@ -293,16 +292,16 @@ export function uiConflicts(context) {
     //         choice(id, keepTheirs, forceRemote)
     //     ]
     // }
-    conflicts.list = function(_) {
-        if (!arguments.length) return conflictList;
-        conflictList = _;
+    conflicts.conflictList = function(_) {
+        if (!arguments.length) return _conflictList;
+        _conflictList = _;
         return conflicts;
     };
 
 
     conflicts.origChanges = function(_) {
-        if (!arguments.length) return origChanges;
-        origChanges = _;
+        if (!arguments.length) return _origChanges;
+        _origChanges = _;
         return conflicts;
     };
 

From ac86869b4a463a7a1d7a77a16f7a739148c92ae3 Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Fri, 5 Jan 2018 18:11:49 -0500
Subject: [PATCH 06/10] Add conflict checking progress, guard code for user
 authentication

---
 data/core.yaml          |  1 +
 dist/locales/en.json    |  1 +
 modules/modes/save.js   | 64 +++++++++++++++++++++++++++++++++++------
 modules/services/osm.js | 16 +++++++++--
 4 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/data/core.yaml b/data/core.yaml
index 9261276af..08021a33f 100644
--- a/data/core.yaml
+++ b/data/core.yaml
@@ -470,6 +470,7 @@ en:
     status_code: "Server returned status code {code}"
     unknown_error_details: "Please ensure you are connected to the internet."
     uploading: Uploading changes to OpenStreetMap...
+    conflict_progress: "Checking for conflicts: {num} of {total}"
     unsaved_changes: You have unsaved changes
     conflict:
       header: Resolve conflicting edits
diff --git a/dist/locales/en.json b/dist/locales/en.json
index 1e13481be..976862a90 100644
--- a/dist/locales/en.json
+++ b/dist/locales/en.json
@@ -582,6 +582,7 @@
             "status_code": "Server returned status code {code}",
             "unknown_error_details": "Please ensure you are connected to the internet.",
             "uploading": "Uploading changes to OpenStreetMap...",
+            "conflict_progress": "Checking for conflicts: {num} of {total}",
             "unsaved_changes": "You have unsaved changes",
             "conflict": {
                 "header": "Resolve conflicting edits",
diff --git a/modules/modes/save.js b/modules/modes/save.js
index 73479e72a..99d4641a0 100644
--- a/modules/modes/save.js
+++ b/modules/modes/save.js
@@ -60,6 +60,9 @@ export function modeSave(context) {
 
     var _toCheck = [];
     var _toLoad = [];
+    var _toLoadCount = 0;
+    var _toLoadTotal = 0;
+
     var _conflicts = [];
     var _errors = [];
     var _origChanges;
@@ -76,13 +79,33 @@ export function modeSave(context) {
 
     function save(changeset, tryAgain, checkConflicts) {
         // Guard against accidentally entering save code twice - #4641
-        if (_isSaving && !tryAgain) return;
+        if (_isSaving && !tryAgain) {
+            return;
+        }
 
         var osm = context.connection();
-        if (!osm) return;
+        if (!osm) {
+            cancel();
+            return;
+        }
 
-        _isSaving = true;
-        context.container().call(loading);  // block input
+        // If user somehow got logged out mid-save, try to reauthenticate..
+        // This can happen if they were logged in from before, but the tokens are no longer valid.
+        if (!osm.authenticated()) {
+            osm.authenticate(function(err) {
+                if (err) {
+                    cancel();   // quit save mode..
+                } else {
+                    save(changeset, tryAgain, checkConflicts);  // continue where we left off..
+                }
+            });
+            return;
+        }
+
+        if (!_isSaving) {
+            context.container().call(loading);  // block input
+            _isSaving = true;
+        }
 
         var history = context.history();
         var localGraph = context.graph();
@@ -109,7 +132,11 @@ export function modeSave(context) {
             var modified = _filter(history.difference().summary(), { changeType: 'modified' });
             _toCheck = _map(_map(modified, 'entity'), 'id');
             _toLoad = withChildNodes(_toCheck, localGraph);
+            _toLoadCount = 0;
+            _toLoadTotal = _toLoad.length;
+
             if (_toCheck.length) {
+                showProgress(_toLoadCount, _toLoadTotal);
                 osm.loadMultiple(_toLoad, loaded);
             } else {
                 upload(changeset);
@@ -142,12 +169,13 @@ export function modeSave(context) {
 
             if (err) {
                 _errors.push({
-                    msg: err.responseText,
+                    msg: err.message || err.responseText,
                     details: [ t('save.status_code', { code: err.status }) ]
                 });
                 showErrors();
 
             } else {
+
                 var loadMore = [];
                 result.data.forEach(function(entity) {
                     remoteGraph.replace(entity);
@@ -165,6 +193,10 @@ export function modeSave(context) {
                     }
                 });
 
+                _toLoadCount += result.data.length;
+                _toLoadTotal += loadMore.length;
+                showProgress(_toLoadCount, _toLoadTotal);
+
                 if (loadMore.length) {
                     _toLoad.push.apply(_toLoad, loadMore);
                     osm.loadMultiple(loadMore, loaded);
@@ -274,7 +306,7 @@ export function modeSave(context) {
                 save(changeset, true, true);   // tryAgain = true, checkConflicts = true
             } else {
                 _errors.push({
-                    msg: err.responseText,
+                    msg: err.message || err.responseText,
                     details: [ t('save.status_code', { code: err.status }) ]
                 });
                 showErrors();
@@ -294,6 +326,20 @@ export function modeSave(context) {
     }
 
 
+    function showProgress(num, total) {
+        var modal = context.container().select('.loading-modal .modal-section');
+        var progress = modal.selectAll('.progress')
+            .data([0]);
+
+        // enter/update
+        progress.enter()
+            .append('div')
+            .attr('class', 'progress')
+            .merge(progress)
+            .text(t('save.conflict_progress', { num: num, total: total }));
+    }
+
+
     function showConflicts(changeset) {
         var history = context.history();
         var selection = context.container()
@@ -425,7 +471,10 @@ export function modeSave(context) {
             .attr('class', 'inactive');
 
         var osm = context.connection();
-        if (!osm) return;
+        if (!osm) {
+            cancel();
+            return;
+        }
 
         if (osm.authenticated()) {
             done();
@@ -442,7 +491,6 @@ export function modeSave(context) {
 
 
     mode.exit = function() {
-        loading.close();
         _isSaving = false;
 
         keybinding.off();
diff --git a/modules/services/osm.js b/modules/services/osm.js
index 89b45013d..9ead1f5b0 100644
--- a/modules/services/osm.js
+++ b/modules/services/osm.js
@@ -253,8 +253,7 @@ export default {
 
             // 400 Bad Request, 401 Unauthorized, 403 Forbidden
             // Logout and retry the request..
-            if (isAuthenticated && err &&
-                    (err.status === 400 || err.status === 401 || err.status === 403)) {
+            if (isAuthenticated && err && (err.status === 400 || err.status === 401 || err.status === 403)) {
                 that.logout();
                 that.loadFromAPI(path, callback);
 
@@ -371,6 +370,10 @@ export default {
             _changeset.inflight = null;
 
             if (err) {
+                // 400 Bad Request, 401 Unauthorized, 403 Forbidden..
+                if (err.status === 400 || err.status === 401 || err.status === 403) {
+                    that.logout();
+                }
                 return callback(err, changeset);
             }
             if (that.getConnectionId() !== cid) {
@@ -428,12 +431,17 @@ export default {
 
         function done(err, user_details) {
             if (err) {
+                // 400 Bad Request, 401 Unauthorized, 403 Forbidden..
+                if (err.status === 400 || err.status === 401 || err.status === 403) {
+                    that.logout();
+                }
                 return callback(err);
             }
             if (that.getConnectionId() !== cid) {
                 return callback({ message: 'Connection Switched', status: -1 });
             }
 
+
             var u = user_details.getElementsByTagName('user')[0];
             var img = u.getElementsByTagName('img');
             var image_url = '';
@@ -482,6 +490,10 @@ export default {
 
             function done(err, changesets) {
                 if (err) {
+                    // 400 Bad Request, 401 Unauthorized, 403 Forbidden..
+                    if (err.status === 400 || err.status === 401 || err.status === 403) {
+                        that.logout();
+                    }
                     return callback(err);
                 }
                 if (that.getConnectionId() !== cid) {

From a22cfe64b8136adfc3045f661cc0d5a8fb8fbe67 Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Sat, 6 Jan 2018 00:30:26 -0500
Subject: [PATCH 07/10] Avoid loading circular relations by storing ids in
 _loaded object (re: #3056)

---
 modules/modes/save.js | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/modules/modes/save.js b/modules/modes/save.js
index 99d4641a0..bb52804a5 100644
--- a/modules/modes/save.js
+++ b/modules/modes/save.js
@@ -1,5 +1,4 @@
 import _clone from 'lodash-es/clone';
-import _difference from 'lodash-es/difference';
 import _filter from 'lodash-es/filter';
 import _map from 'lodash-es/map';
 import _reduce from 'lodash-es/reduce';
@@ -60,6 +59,7 @@ export function modeSave(context) {
 
     var _toCheck = [];
     var _toLoad = [];
+    var _loaded = {};
     var _toLoadCount = 0;
     var _toLoadTotal = 0;
 
@@ -132,11 +132,13 @@ export function modeSave(context) {
             var modified = _filter(history.difference().summary(), { changeType: 'modified' });
             _toCheck = _map(_map(modified, 'entity'), 'id');
             _toLoad = withChildNodes(_toCheck, localGraph);
+            _loaded = {};
             _toLoadCount = 0;
             _toLoadTotal = _toLoad.length;
 
             if (_toCheck.length) {
                 showProgress(_toLoadCount, _toLoadTotal);
+                _toLoad.forEach(function(id) { _loaded[id] = false; });
                 osm.loadMultiple(_toLoad, loaded);
             } else {
                 upload(changeset);
@@ -163,6 +165,7 @@ export function modeSave(context) {
             }, _clone(ids)));
         }
 
+
         // Reload modified entities into an alternate graph and check for conflicts..
         function loaded(err, result) {
             if (_errors.length) return;
@@ -175,21 +178,34 @@ export function modeSave(context) {
                 showErrors();
 
             } else {
-
                 var loadMore = [];
+
                 result.data.forEach(function(entity) {
                     remoteGraph.replace(entity);
+                    _loaded[entity.id] = true;
                     _toLoad = _without(_toLoad, entity.id);
 
+                    if (!entity.visible) return;
+
                     // Because loadMultiple doesn't download /full like loadEntity,
                     // need to also load children that aren't already being checked..
-                    if (!entity.visible) return;
+                    var i, id;
                     if (entity.type === 'way') {
-                        loadMore.push.apply(loadMore,
-                            _difference(entity.nodes, _toCheck, _toLoad, loadMore));
+                        for (i = 0; i < entity.nodes.length; i++) {
+                            id = entity.nodes[i];
+                            if (_loaded[id] === undefined) {
+                                _loaded[id] = false;
+                                loadMore.push(id);
+                            }
+                        }
                     } else if (entity.type === 'relation' && entity.isMultipolygon()) {
-                        loadMore.push.apply(loadMore,
-                            _difference(_map(entity.members, 'id'), _toCheck, _toLoad, loadMore));
+                        for (i = 0; i < entity.members.length; i++) {
+                            id = entity.members[i].id;
+                            if (_loaded[id] === undefined) {
+                                _loaded[id] = false;
+                                loadMore.push(id);
+                            }
+                        }
                     }
                 });
 

From 590487d23703cd58d313bc255f130b3930d3669d Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Sat, 6 Jan 2018 23:16:48 -0500
Subject: [PATCH 08/10] Fix escape keybind when conflicts ui is active (re:
 4351)

---
 modules/modes/save.js   | 28 +++++++++++++++++++---------
 modules/ui/conflicts.js | 38 ++++++++++++++++++++++++++++++++------
 2 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/modules/modes/save.js b/modules/modes/save.js
index bb52804a5..f5198b41b 100644
--- a/modules/modes/save.js
+++ b/modules/modes/save.js
@@ -47,7 +47,7 @@ var _isSaving = false;
 
 export function modeSave(context) {
     var mode = { id: 'save' };
-    var keybinding = d3_keybinding('select');
+    var keybinding = d3_keybinding('save');
 
     var loading = uiLoading(context)
         .message(t('save.uploading'))
@@ -103,6 +103,7 @@ export function modeSave(context) {
         }
 
         if (!_isSaving) {
+            keybindingOff();
             context.container().call(loading);  // block input
             _isSaving = true;
         }
@@ -372,6 +373,7 @@ export function modeSave(context) {
             .on('cancel', function() {
                 history.pop();
                 selection.remove();
+                keybindingOn();
             })
             .on('save', function() {
                 for (var i = 0; i < _conflicts.length; i++) {
@@ -396,12 +398,12 @@ export function modeSave(context) {
 
 
     function showErrors() {
-        var selection = uiConfirm(context.container());
-
+        keybindingOn();
         context.history().pop();
         loading.close();
         _isSaving = false;
 
+        var selection = uiConfirm(context.container());
         selection
             .select('.modal-section.header')
             .append('h3')
@@ -472,16 +474,24 @@ export function modeSave(context) {
     }
 
 
+    function keybindingOn() {
+        d3_select(document)
+            .call(keybinding.on('⎋', cancel, true));
+    }
+
+
+    function keybindingOff() {
+        d3_select(document)
+            .call(keybinding.off);
+    }
+
+
     mode.enter = function() {
         function done() {
             context.ui().sidebar.show(commit);
         }
 
-        keybinding
-            .on('⎋', cancel, true);
-
-        d3_select(document)
-            .call(keybinding);
+        keybindingOn();
 
         context.container().selectAll('#content')
             .attr('class', 'inactive');
@@ -509,7 +519,7 @@ export function modeSave(context) {
     mode.exit = function() {
         _isSaving = false;
 
-        keybinding.off();
+        keybindingOff();
 
         context.container().selectAll('#content')
             .attr('class', 'active');
diff --git a/modules/ui/conflicts.js b/modules/ui/conflicts.js
index 21a0aa2b7..2180b235e 100644
--- a/modules/ui/conflicts.js
+++ b/modules/ui/conflicts.js
@@ -5,6 +5,8 @@ import {
     select as d3_select
 } from 'd3-selection';
 
+import { d3keybinding as d3_keybinding } from '../lib/d3.keybinding.js';
+
 import { t } from '../util/locale';
 import { JXON } from '../util/jxon';
 import { geoExtent } from '../geo';
@@ -17,11 +19,35 @@ import { utilRebind } from '../util/rebind';
 
 export function uiConflicts(context) {
     var dispatch = d3_dispatch('cancel', 'save');
+    var keybinding = d3_keybinding('conflicts');
     var _origChanges;
     var _conflictList;
 
 
+    function keybindingOn() {
+        d3_select(document)
+            .call(keybinding.on('⎋', cancel, true));
+    }
+
+    function keybindingOff() {
+        d3_select(document)
+            .call(keybinding.off);
+    }
+
+    function tryAgain() {
+        keybindingOff();
+        dispatch.call('save');
+    }
+
+    function cancel() {
+        keybindingOff();
+        dispatch.call('cancel');
+    }
+
+
     function conflicts(selection) {
+        keybindingOn();
+
         var header = selection
             .append('div')
             .attr('class', 'header fillL');
@@ -29,7 +55,7 @@ export function uiConflicts(context) {
         header
             .append('button')
             .attr('class', 'fr')
-            .on('click', function() { dispatch.call('cancel'); })
+            .on('click', cancel)
             .call(svgIcon('#icon-close'));
 
         header
@@ -102,13 +128,13 @@ export function uiConflicts(context) {
             .attr('disabled', _conflictList.length > 1)
             .attr('class', 'action conflicts-button col6')
             .text(t('save.title'))
-            .on('click.try_again', function() { dispatch.call('save'); });
+            .on('click.try_again', tryAgain);
 
         buttons
             .append('button')
             .attr('class', 'secondary-action conflicts-button col6')
             .text(t('confirm.cancel'))
-            .on('click.cancel', function() { dispatch.call('cancel'); });
+            .on('click.cancel', cancel);
     }
 
 
@@ -149,8 +175,8 @@ export function uiConflicts(context) {
             .attr('href', '#')
             .text(function(d) { return d.name; })
             .on('click', function(d) {
-                zoomToEntity(d.id);
                 d3_event.preventDefault();
+                zoomToEntity(d.id);
             });
 
         var details = enter
@@ -186,6 +212,8 @@ export function uiConflicts(context) {
                     (i === 1 && index === _conflictList.length - 1) || null;
             })
             .on('click', function(d, i) {
+                d3_event.preventDefault();
+
                 var container = parent.select('.conflict-container');
                 var sign = (i === 0 ? -1 : 1);
 
@@ -195,8 +223,6 @@ export function uiConflicts(context) {
 
                 container
                     .call(showConflict, index + sign);
-
-                d3_event.preventDefault();
             });
 
         item.exit()

From 1b973f3c9bd01c531a7326f288f76ba265e48d90 Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Sun, 7 Jan 2018 23:46:54 -0500
Subject: [PATCH 09/10] Be more careful about enter/update selection in
 conflict resolution ui

This fixes one of the issues in #4351 where the radio button was not
selected. This was likely introduced during the upgrade to d3 v4, now
that enter selections do not automatically flow into update anymore.
(the fix is to add a `merge` to ensure that the `selection.each` actually
has some things to iterate over)
---
 modules/ui/conflicts.js | 72 ++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 30 deletions(-)

diff --git a/modules/ui/conflicts.js b/modules/ui/conflicts.js
index 2180b235e..885b05479 100644
--- a/modules/ui/conflicts.js
+++ b/modules/ui/conflicts.js
@@ -13,8 +13,12 @@ import { geoExtent } from '../geo';
 import { osmChangeset } from '../osm';
 import { svgIcon } from '../svg';
 import { utilDetect } from '../util/detect';
-import { utilEntityOrMemberSelector } from '../util';
-import { utilRebind } from '../util/rebind';
+
+import {
+    utilEntityOrMemberSelector,
+    utilRebind,
+    utilWrap
+} from '../util';
 
 
 export function uiConflicts(context) {
@@ -48,25 +52,29 @@ export function uiConflicts(context) {
     function conflicts(selection) {
         keybindingOn();
 
-        var header = selection
+        var headerEnter = selection.selectAll('.header')
+            .data([0])
+            .enter()
             .append('div')
             .attr('class', 'header fillL');
 
-        header
+        headerEnter
             .append('button')
             .attr('class', 'fr')
             .on('click', cancel)
             .call(svgIcon('#icon-close'));
 
-        header
+        headerEnter
             .append('h3')
             .text(t('save.conflict.header'));
 
-        var body = selection
+        var bodyEnter = selection.selectAll('.body')
+            .data([0])
+            .enter()
             .append('div')
             .attr('class', 'body fillL');
 
-        var conflictsHelp = body
+        var conflictsHelpEnter = bodyEnter
             .append('div')
             .attr('class', 'conflicts-help')
             .text(t('save.conflict.help'));
@@ -82,9 +90,7 @@ export function uiConflicts(context) {
         var blob = new Blob([data], { type: 'text/xml;charset=utf-8;' });
         var fileName = 'changes.osc';
 
-        var linkEnter = conflictsHelp.selectAll('.download-changes')
-            .data([0])
-            .enter()
+        var linkEnter = conflictsHelpEnter.selectAll('.download-changes')
             .append('a')
             .attr('class', 'download-changes');
 
@@ -107,30 +113,30 @@ export function uiConflicts(context) {
             .text(t('save.conflict.download_changes'));
 
 
-        body
+        bodyEnter
             .append('div')
             .attr('class', 'conflict-container fillL3')
             .call(showConflict, 0);
 
-        body
+        bodyEnter
             .append('div')
             .attr('class', 'conflicts-done')
             .attr('opacity', 0)
             .style('display', 'none')
             .text(t('save.conflict.done'));
 
-        var buttons = body
+        var buttonsEnter = bodyEnter
             .append('div')
             .attr('class','buttons col12 joined conflicts-buttons');
 
-        buttons
+        buttonsEnter
             .append('button')
             .attr('disabled', _conflictList.length > 1)
             .attr('class', 'action conflicts-button col6')
             .text(t('save.title'))
             .on('click.try_again', tryAgain);
 
-        buttons
+        buttonsEnter
             .append('button')
             .attr('class', 'secondary-action conflicts-button col6')
             .text(t('confirm.cancel'))
@@ -139,7 +145,7 @@ export function uiConflicts(context) {
 
 
     function showConflict(selection, index) {
-        if (index < 0 || index >= _conflictList.length) return;
+        index = utilWrap(index, _conflictList.length);
 
         var parent = d3_select(selection.node().parentNode);
 
@@ -156,20 +162,23 @@ export function uiConflicts(context) {
             }, 250);
         }
 
-        var item = selection
+        var conflict = selection
             .selectAll('.conflict')
             .data([_conflictList[index]]);
 
-        var enter = item.enter()
+        conflict.exit()
+            .remove();
+
+        var conflictEnter = conflict.enter()
             .append('div')
             .attr('class', 'conflict');
 
-        enter
+        conflictEnter
             .append('h4')
             .attr('class', 'conflict-count')
             .text(t('save.conflict.count', { num: index + 1, total: _conflictList.length }));
 
-        enter
+        conflictEnter
             .append('a')
             .attr('class', 'conflict-description')
             .attr('href', '#')
@@ -179,7 +188,7 @@ export function uiConflicts(context) {
                 zoomToEntity(d.id);
             });
 
-        var details = enter
+        var details = conflictEnter
             .append('div')
             .attr('class', 'conflict-detail-container');
 
@@ -214,7 +223,7 @@ export function uiConflicts(context) {
             .on('click', function(d, i) {
                 d3_event.preventDefault();
 
-                var container = parent.select('.conflict-container');
+                var container = parent.selectAll('.conflict-container');
                 var sign = (i === 0 ? -1 : 1);
 
                 container
@@ -225,8 +234,6 @@ export function uiConflicts(context) {
                     .call(showConflict, index + sign);
             });
 
-        item.exit()
-            .remove();
     }
 
 
@@ -237,14 +244,15 @@ export function uiConflicts(context) {
             .selectAll('li')
             .data(function(d) { return d.choices || []; });
 
-        var enter = choices.enter()
+        // enter
+        var choicesEnter = choices.enter()
             .append('li')
             .attr('class', 'layer');
 
-        var label = enter
+        var labelEnter = choicesEnter
             .append('label');
 
-        label
+        labelEnter
             .append('input')
             .attr('type', 'radio')
             .attr('name', function(d) { return d.id; })
@@ -254,14 +262,18 @@ export function uiConflicts(context) {
                 choose(ul, d);
             });
 
-        label
+        labelEnter
             .append('span')
             .text(function(d) { return d.text; });
 
-        choices
+        // update
+        choicesEnter
+            .merge(choices)
             .each(function(d, i) {
                 var ul = this.parentNode;
-                if (ul.__data__.chosen === i) choose(ul, d);
+                if (ul.__data__.chosen === i) {
+                    choose(ul, d);
+                }
             });
     }
 

From 8472f99347da3eedc0aa2bdf99dcb38ceaeb9df3 Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Mon, 8 Jan 2018 09:49:32 -0500
Subject: [PATCH 10/10] Zooming out should not exit save mode (closes #4664)

---
 modules/renderer/map.js | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/modules/renderer/map.js b/modules/renderer/map.js
index a4ebac8a4..bac563eeb 100644
--- a/modules/renderer/map.js
+++ b/modules/renderer/map.js
@@ -344,7 +344,12 @@ export function rendererMap(context) {
     function editOff() {
         context.features().resetStats();
         surface.selectAll('.layer-osm *').remove();
-        context.enter(modeBrowse(context));
+
+        var mode = context.mode();
+        if (mode && mode.id !== 'save') {
+            context.enter(modeBrowse(context));
+        }
+
         dispatch.call('drawn', this, {full: true});
     }