Fix issue where merging a point matching a fallback preset into an area could discard tag values (close #7446)

modules/actions/merge.js

@@ -1,3 +1,4 @@
+import { osmTagSuggestingArea } from '../osm/tags';
 import { utilArrayGroupBy, utilArrayUniq } from '../util';
 
 
@@ -48,6 +49,16 @@ export function actionMerge(ids) {
             graph = graph.remove(removeNode);
         });
 
+        if (target.tags.area) {
+            var tags = Object.assign({}, target.tags); // shallow copy
+            delete tags.area;
+            if (osmTagSuggestingArea(tags)) {
+                // remove the `area` tag if area geometry is now implied - #3851
+                target = target.update({ tags: tags });
+                graph = graph.replace(target);
+            }
+        }
+
         return graph;
     };
 

modules/operations/merge.js

@@ -1,6 +1,5 @@
 import { t } from '../util/locale';
 
-import { actionChangePreset } from '../actions/change_preset';
 import { actionJoin } from '../actions/join';
 import { actionMerge } from '../actions/merge';
 import { actionMergeNodes } from '../actions/merge_nodes';
@@ -12,62 +11,39 @@ import { modeSelect } from '../modes/select';
 
 export function operationMerge(selectedIDs, context) {
 
-    function updatePresetTags(newGraph, ids) {
-        var id = ids[0];
-        var newEntity = newGraph.hasEntity(id);
-
-        if (!newEntity) return;
-        var newPreset = context.presets().match(newEntity, newGraph);
-        context.replace(actionChangePreset(id, null, newPreset), operation.annotation());
-    }
-
-
     var join = actionJoin(selectedIDs);
     var merge = actionMerge(selectedIDs);
     var mergePolygon = actionMergePolygon(selectedIDs);
     var mergeNodes = actionMergeNodes(selectedIDs);
 
+    function getAction() {
+        if (!join.disabled(context.graph())) {
+            return join;
+
+        } else if (!merge.disabled(context.graph())) {
+            return merge;
+            
+        } else if (!mergePolygon.disabled(context.graph())) {
+            return mergePolygon;
+        }
+        return mergeNodes;
+    }
 
     var operation = function() {
-        var doUpdateTags;
-        var action;
-
-        if (!join.disabled(context.graph())) {
-            doUpdateTags = false;
-            action = join;
-        } else if (!merge.disabled(context.graph())) {
-            doUpdateTags = true;
-            action = merge;
-        } else if (!mergePolygon.disabled(context.graph())) {
-            doUpdateTags = false;
-            action = mergePolygon;
-        } else {
-            doUpdateTags = true;
-            action = mergeNodes;
-        }
+        var action = getAction();
 
         context.perform(action, operation.annotation());
 
-        var ids = selectedIDs.filter(function(id) {
-            var entity = context.hasEntity(id);
-            return entity && entity.type !== 'node';
-        });
-
-        // if we merged tags, rematch preset to update tags if necessary (#3851)
-        if (doUpdateTags) {
-            updatePresetTags(context.graph(), ids);
-        }
-
         context.validator().validate();
-        context.enter(modeSelect(context, ids));
-    };
 
+        var resultIDs = selectedIDs.filter(context.hasEntity);
+        context.enter(modeSelect(context, resultIDs));
+    };
 
     operation.available = function() {
         return selectedIDs.length >= 2;
     };
 
-
     operation.disabled = function() {
         return join.disabled(context.graph()) &&
             merge.disabled(context.graph()) &&
@@ -75,7 +51,6 @@ export function operationMerge(selectedIDs, context) {
             mergeNodes.disabled(context.graph());
     };
 
-
     operation.tooltip = function() {
         var j = join.disabled(context.graph());          // 'not_eligible', 'not_adjacent', 'restriction', 'conflicting_tags'
         var m = merge.disabled(context.graph());         // 'not_eligible'
@@ -103,12 +78,10 @@ export function operationMerge(selectedIDs, context) {
         }
     };
 
-
     operation.annotation = function() {
         return t('operations.merge.annotation', { n: selectedIDs.length });
     };
 
-
     operation.id = 'merge';
     operation.keys = [t('operations.merge.key')];
     operation.title = t('operations.merge.title');

modules/osm/index.js

@@ -26,6 +26,7 @@ export {
 export {
     osmAreaKeys,
     osmSetAreaKeys,
+    osmTagSuggestingArea,
     osmPointTags,
     osmSetPointTags,
     osmVertexTags,

modules/osm/tags.js

@@ -13,6 +13,41 @@ export function osmSetAreaKeys(value) {
     osmAreaKeys = value;
 }
 
+// returns an object with the tag from `tags` that implies an area geometry, if any
+export function osmTagSuggestingArea(tags) {
+    if (tags.area === 'yes') return { area: 'yes' };
+    if (tags.area === 'no') return null;
+
+    // `highway` and `railway` are typically linear features, but there
+    // are a few exceptions that should be treated as areas, even in the
+    // absence of a proper `area=yes` or `areaKeys` tag.. see #4194
+    var lineKeys = {
+        highway: {
+            rest_area: true,
+            services: true
+        },
+        railway: {
+            roundhouse: true,
+            station: true,
+            traverser: true,
+            turntable: true,
+            wash: true
+        }
+    };
+    var returnTags = {};
+    for (var key in tags) {
+        if (key in osmAreaKeys && !(tags[key] in osmAreaKeys[key])) {
+            returnTags[key] = tags[key];
+            return returnTags;
+        }
+        if (key in lineKeys && tags[key] in lineKeys[key]) {
+            returnTags[key] = tags[key];
+            return returnTags;
+        }
+    }
+    return null;
+}
+
 // Tags that indicate a node can be a standalone point
 // e.g. { amenity: { bar: true, parking: true, ... } ... }
 export var osmPointTags = {};

modules/osm/way.js

@@ -3,7 +3,7 @@ import { geoArea as d3_geoArea } from 'd3-geo';
 import { geoExtent, geoVecCross } from '../geo';
 import { osmEntity } from './entity';
 import { osmLanes } from './lanes';
-import { osmAreaKeys, osmOneWayTags, osmRightSideIsInsideTags } from './tags';
+import { osmTagSuggestingArea, osmOneWayTags, osmRightSideIsInsideTags } from './tags';
 import { utilArrayUniq } from '../util';
 
 
@@ -228,37 +228,7 @@ Object.assign(osmWay.prototype, {
 
     // returns an object with the tag that implies this is an area, if any
     tagSuggestingArea: function() {
-        if (this.tags.area === 'yes') return { area: 'yes' };
-        if (this.tags.area === 'no') return null;
-
-        // `highway` and `railway` are typically linear features, but there
-        // are a few exceptions that should be treated as areas, even in the
-        // absence of a proper `area=yes` or `areaKeys` tag.. see #4194
-        var lineKeys = {
-            highway: {
-                rest_area: true,
-                services: true
-            },
-            railway: {
-                roundhouse: true,
-                station: true,
-                traverser: true,
-                turntable: true,
-                wash: true
-            }
-        };
-        var returnTags = {};
-        for (var key in this.tags) {
-            if (key in osmAreaKeys && !(this.tags[key] in osmAreaKeys[key])) {
-                returnTags[key] = this.tags[key];
-                return returnTags;
-            }
-            if (key in lineKeys && this.tags[key] in lineKeys[key]) {
-                returnTags[key] = this.tags[key];
-                return returnTags;
-            }
-        }
-        return null;
+        return osmTagSuggestingArea(this.tags);
     },
 
     isArea: function() {