Index: src/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerAction.java =================================================================== --- src/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerAction.java (revision 18892) +++ src/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerAction.java (working copy) @@ -47,8 +47,6 @@ return; if (!SaveLayersDialog.saveUnsavedModifications(selectedLayers, SaveLayersDialog.Reason.DELETE)) return; - final Map layerMap = model.selectedIndices().filter(i -> model.getLayer(i) != null) - .collect(HashMap::new, (map, value) -> map.put(value, model.getLayer(value)), HashMap::putAll); for (Layer l: selectedLayers) { if (model.getLayerManager().containsLayer(l)) { // it may happen that this call removes other layers. @@ -56,31 +54,6 @@ model.getLayerManager().removeLayer(l); } } - // Set the next active layer to the next visible layer - if (layerMap.size() == 1) { - final int selected = Math.min(layerMap.keySet().iterator().next(), model.getRowCount() - 1); - int currentLayerIndex = selected; - Layer layer = model.getLayer(currentLayerIndex); - // If the user has the last layer selected, we need to wrap around. - boolean reversed = false; - while (layer != null && !layer.isVisible() && currentLayerIndex < model.getRowCount() && currentLayerIndex >= 0) { - if (reversed) { - currentLayerIndex--; - } else { - currentLayerIndex++; - } - if (currentLayerIndex == model.getRowCount()) { - reversed = true; - currentLayerIndex = selected; - } - layer = model.getLayer(currentLayerIndex); - } - if (layer != null) { - model.getLayerManager().setActiveLayer(layer); - // Reset the selection - model.getSelectionModel().setSelectionInterval(selected, selected); - } - } } @Override Index: test/unit/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerActionTest.java =================================================================== --- test/unit/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerActionTest.java (revision 18892) +++ test/unit/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerActionTest.java (working copy) @@ -10,6 +10,7 @@ import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.Collections; import java.util.Objects; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; @@ -21,6 +22,7 @@ import org.openstreetmap.josm.gui.dialogs.LayerListDialog; import org.openstreetmap.josm.gui.layer.Layer; import org.openstreetmap.josm.gui.layer.OsmDataLayer; +import org.openstreetmap.josm.gui.layer.geoimage.GeoImageLayer; import org.openstreetmap.josm.testutils.annotations.Main; import org.openstreetmap.josm.testutils.annotations.Projection; @@ -122,6 +124,47 @@ assertAll(model.getLayers().stream().map(layer -> () -> assertNotSame(toRemove, layer))); } + @Test + void testRemoveBottomActiveWithBackgroundLayer() { + GeoImageLayer geoImageLayer = new GeoImageLayer(Collections.emptyList(), null, "imageLayer"); + OsmDataLayer osmDataLayer1 = new OsmDataLayer(new DataSet(), "dataLayer1", null); + OsmDataLayer osmDataLayer2 = new OsmDataLayer(new DataSet(), "dataLayer2", null); + + // remove all the layers added in BeforeEach() + for (Layer l : MainApplication.getLayerManager().getLayers()) { + MainApplication.getLayerManager().removeLayer(l); + } + MainApplication.getLayerManager().addLayer(geoImageLayer); + MainApplication.getLayerManager().addLayer(osmDataLayer1); + MainApplication.getLayerManager().addLayer(osmDataLayer2); + + model.getLayerManager().setActiveLayer(osmDataLayer1); + model.setSelectedLayer(osmDataLayer1); + + deleteLayerAction.actionPerformed(null); + + assertSame(model.getLayerManager().getActiveLayer(), model.getLayer(0)); + assertEquals("dataLayer2", Objects.requireNonNull(model.getLayerManager().getActiveLayer().getName())); + assertAll(model.getLayers().stream().map(layer -> () -> assertNotSame(osmDataLayer1, layer))); + } + + @Test + void testRemoveBottomActiveAllHidden() { + hideRange(0, 9); + final Layer toRemove = model.getLayer(9); + assertNotNull(toRemove); + assertFalse(toRemove.isVisible()); + assertEquals(0, model.getLayers().stream().filter(Layer::isVisible).count()); + + model.getLayerManager().setActiveLayer(toRemove); + model.setSelectedLayer(toRemove); + deleteLayerAction.actionPerformed(null); + + assertSame(model.getLayerManager().getActiveLayer(), model.getLayer(8)); + assertEquals("testActiveLayer1", Objects.requireNonNull(model.getLayer(8)).getName()); + assertAll(model.getLayers().stream().map(layer -> () -> assertNotSame(toRemove, layer))); + } + private void hideRange(int start, int end) { model.getSelectionModel().setSelectionInterval(start, end); showHideLayerAction.actionPerformed(null); Index: src/org/openstreetmap/josm/gui/layer/Layer.java =================================================================== --- src/org/openstreetmap/josm/gui/layer/Layer.java (revision 18892) +++ src/org/openstreetmap/josm/gui/layer/Layer.java (working copy) @@ -35,14 +35,14 @@ /** * A layer encapsulates the gui component of one dataset and its representation. - * + *

* Some layers may display data directly imported from OSM server. Other only * display background images. Some can be edited, some not. Some are static and * other changes dynamically (auto-updated). - * + *

* Layers can be visible or not. Most actions the user can do applies only on * selected layers. The available actions depend on the selected layers too. - * + *

* All layers are managed by the MapView. They are displayed in a list to the * right of the screen. * @@ -174,12 +174,12 @@ /** * Initialization code, that depends on Main.map.mapView. - * + *

* It is always called in the event dispatching thread. * Note that Main.map is null as long as no layer has been added, so do * not execute code in the constructor, that assumes Main.map.mapView is * not null. - * + *

* If you need to execute code when this layer is added to the map view, use * {@link #attachToMapView(org.openstreetmap.josm.gui.layer.MapViewPaintable.MapViewEvent)} */ @@ -269,7 +269,7 @@ * Returns list of actions. Action can implement LayerAction interface when it needs to be represented by other * menu component than JMenuItem or when it supports multiple layers. Actions that support multiple layers should also * have correct equals implementation. - * + *

* Use {@link SeparatorLayerAction#INSTANCE} instead of new JSeparator * @return menu actions for this layer */ @@ -277,8 +277,8 @@ /** * Called, when the layer is removed from the mapview and is going to be destroyed. - * - * This is because the Layer constructor can not add itself safely as listener + *

+ * This is because the Layer constructor cannot add itself safely as a listener * to the layerlist dialog, because there may be no such dialog yet (loaded * via command line parameter). */ @@ -302,7 +302,7 @@ /** * Sets the associated file for this layer. - * + *

* The associated file might be the one that the user opened. * @param file The file, may be null */ @@ -353,9 +353,9 @@ } /** - * Replies true if this layer was renamed by user + * Replies true if user renamed this layer * - * @return true if this layer was renamed by user + * @return true if user renamed this layer */ public boolean isRenamed() { return renamed; @@ -483,7 +483,7 @@ } /** - * fires a property change for the property {@link #FILTER_STATE_PROP}. + * Fires a property change for the property {@link #FILTER_STATE_PROP}. */ protected void fireFilterStateChanged() { propertyChangeSupport.firePropertyChange(FILTER_STATE_PROP, null, null); @@ -490,9 +490,8 @@ } /** - * allows to check whether a projection is supported or not + * Allows to check whether a projection is supported or not. * @param proj projection - * * @return True if projection is supported for this layer */ public boolean isProjectionSupported(Projection proj) { @@ -609,7 +608,7 @@ } /** - * Replies the savable state of this layer (i.e if it can be saved through a "File->Save" dialog). + * Replies the savable state of this layer (i.e., if it can be saved through a "File->Save" dialog). * @return true if this layer can be saved to a file * @since 5459 */ @@ -627,8 +626,9 @@ } /** - * Creates a new "Save" dialog for this layer and makes it visible.
- * When the user has chosen a file, checks the file extension, and confirms overwrite if needed. + * Creates a new "Save" dialog for this layer and makes it visible. + *

+ * When the user has chosen a file, checks the file extension, and confirms overwriting if needed. * @return The output {@code File} * @see SaveActionBase#createAndOpenSaveFileChooser * @since 5459 @@ -674,6 +674,6 @@ @Override public String toString() { - return getClass().getSimpleName() + " [name=" + name + ", associatedFile=" + associatedFile + ']'; + return getClass().getSimpleName() + " [name=" + name + ", associatedFile=" + associatedFile + ", visible=" + visible + ']'; } } Index: src/org/openstreetmap/josm/gui/layer/MainLayerManager.java =================================================================== --- src/org/openstreetmap/josm/gui/layer/MainLayerManager.java (revision 18892) +++ src/org/openstreetmap/josm/gui/layer/MainLayerManager.java (working copy) @@ -6,6 +6,7 @@ import java.awt.GraphicsEnvironment; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.ListIterator; import java.util.concurrent.CopyOnWriteArrayList; @@ -357,13 +358,15 @@ } /** - * Determines the next active data layer according to the following - * rules: - *

+ * Determines the next active data layer. + *

+ * The layer becomes active, which has the next highest index (closer to bottom) relative to {@code except} parameter + * in the following order: + *

    + *
  1. {@link OsmDataLayer} and visible, or if there is none
    2. + *
  2. {@link OsmDataLayer} and hidden, or if there is none
    3. + *
  3. any type
    4. + *
* * @param except A layer to ignore. * @return the next active data layer @@ -370,9 +373,24 @@ */ private Layer suggestNextActiveLayer(Layer except) { List layersList = new ArrayList<>(getLayers()); - layersList.remove(except); - // First look for data layer - for (Layer layer : layersList) { + + // construct a new list with decreasing priority + int indexOfExcept = layersList.indexOf(except); + List layersList2 = new ArrayList<>(layersList.subList(indexOfExcept, layersList.size())); + List layersList3 = new ArrayList<>(layersList.subList(0, indexOfExcept)); + Collections.reverse(layersList3); + layersList2.addAll(layersList3); + layersList2.remove(except); + + // First look for visible data layer + for (Layer layer : layersList2) { + if (layer instanceof OsmDataLayer && layer.isVisible()) { + return layer; + } + } + + // Then any data layer + for (Layer layer : layersList2) { if (layer instanceof OsmDataLayer) { return layer; } @@ -379,8 +397,8 @@ } // Then any layer - if (!layersList.isEmpty()) - return layersList.get(0); + if (!layersList2.isEmpty()) + return layersList2.get(0); // and then give up return null;