Index: /trunk/src/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerAction.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerAction.java (revision 18894) +++ /trunk/src/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerAction.java (revision 18895) @@ -6,7 +6,5 @@ import java.awt.Component; import java.awt.event.ActionEvent; -import java.util.HashMap; import java.util.List; -import java.util.Map; import javax.swing.AbstractAction; @@ -48,6 +46,4 @@ 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)) { @@ -55,29 +51,4 @@ // this is why we need to check if every layer is still in the list of selected layers. 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); } } Index: /trunk/src/org/openstreetmap/josm/gui/layer/Layer.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/layer/Layer.java (revision 18894) +++ /trunk/src/org/openstreetmap/josm/gui/layer/Layer.java (revision 18895) @@ -36,12 +36,12 @@ /** * 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. @@ -175,10 +175,10 @@ /** * 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)} @@ -270,5 +270,5 @@ * 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 @@ -278,6 +278,6 @@ /** * 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). @@ -303,5 +303,5 @@ /** * 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 @@ -354,7 +354,7 @@ /** - * Replies true if this layer was renamed by user - * - * @return true if this layer was renamed by user + * Replies true if user renamed this layer + * + * @return true if user renamed this layer */ public boolean isRenamed() { @@ -484,5 +484,5 @@ /** - * 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() { @@ -491,7 +491,6 @@ /** - * 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 */ @@ -610,5 +609,5 @@ /** - * 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 @@ -628,6 +627,7 @@ /** - * 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 @@ -675,5 +675,5 @@ @Override public String toString() { - return getClass().getSimpleName() + " [name=" + name + ", associatedFile=" + associatedFile + ']'; + return getClass().getSimpleName() + " [name=" + name + ", associatedFile=" + associatedFile + ", visible=" + visible + ']'; } } Index: /trunk/src/org/openstreetmap/josm/gui/layer/MainLayerManager.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/layer/MainLayerManager.java (revision 18894) +++ /trunk/src/org/openstreetmap/josm/gui/layer/MainLayerManager.java (revision 18895) @@ -7,4 +7,5 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.ListIterator; @@ -358,11 +359,13 @@ /** - * 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. @@ -371,7 +374,22 @@ 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 remainingLayers = new ArrayList<>(layersList.subList(indexOfExcept, layersList.size())); + List previousLayers = new ArrayList<>(layersList.subList(0, indexOfExcept)); + Collections.reverse(previousLayers); + remainingLayers.addAll(previousLayers); + remainingLayers.remove(except); + + // First look for visible data layer + for (Layer layer : remainingLayers) { + if (layer instanceof OsmDataLayer && layer.isVisible()) { + return layer; + } + } + + // Then any data layer + for (Layer layer : remainingLayers) { if (layer instanceof OsmDataLayer) { return layer; @@ -380,6 +398,9 @@ // Then any layer - if (!layersList.isEmpty()) - return layersList.get(0); + for (Layer layer : layersList) { + if (layer != except) { + return layer; + } + } // and then give up Index: /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerActionTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerActionTest.java (revision 18894) +++ /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerActionTest.java (revision 18895) @@ -11,4 +11,5 @@ import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.Collections; import java.util.Objects; import java.util.concurrent.atomic.AtomicInteger; @@ -22,4 +23,5 @@ 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; @@ -123,4 +125,45 @@ } + @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); Index: /trunk/test/unit/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayerTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayerTest.java (revision 18894) +++ /trunk/test/unit/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayerTest.java (revision 18895) @@ -6,4 +6,5 @@ import java.util.Collections; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.openstreetmap.josm.data.osm.DataSet; @@ -19,4 +20,11 @@ @Main class GeoImageLayerTest { + @AfterEach + void tearDown() { + if (ImageViewerDialog.hasInstance()) { + ImageViewerDialog.getInstance().destroy(); + } + } + /** * Test that {@link GeoImageLayer#mergeFrom} throws IAE for invalid arguments Index: /trunk/test/unit/org/openstreetmap/josm/gui/layer/geoimage/ImagesLoaderTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/layer/geoimage/ImagesLoaderTest.java (revision 18894) +++ /trunk/test/unit/org/openstreetmap/josm/gui/layer/geoimage/ImagesLoaderTest.java (revision 18895) @@ -16,4 +16,6 @@ import org.openstreetmap.josm.io.GpxReader; import org.openstreetmap.josm.testutils.annotations.BasicPreferences; +import org.openstreetmap.josm.testutils.annotations.Main; +import org.openstreetmap.josm.testutils.annotations.Projection; /** @@ -21,4 +23,6 @@ */ @BasicPreferences +@Main +@Projection class ImagesLoaderTest { /** Index: /trunk/test/unit/org/openstreetmap/josm/gui/layer/geoimage/WikimediaCommonsLoaderTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/layer/geoimage/WikimediaCommonsLoaderTest.java (revision 18894) +++ /trunk/test/unit/org/openstreetmap/josm/gui/layer/geoimage/WikimediaCommonsLoaderTest.java (revision 18895) @@ -11,10 +11,10 @@ import java.util.List; -import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.openstreetmap.josm.data.Bounds; import org.openstreetmap.josm.testutils.annotations.BasicPreferences; -import org.openstreetmap.josm.tools.Http1Client; -import org.openstreetmap.josm.tools.HttpClient; +import org.openstreetmap.josm.testutils.annotations.HTTP; +import org.openstreetmap.josm.testutils.annotations.Main; import com.github.tomakehurst.wiremock.WireMockServer; @@ -24,9 +24,12 @@ */ @BasicPreferences +@HTTP +@Main class WikimediaCommonsLoaderTest { - - @BeforeAll - static void beforeAll() { - HttpClient.setFactory(Http1Client::new); + @AfterEach + void tearDown() { + if (ImageViewerDialog.hasInstance()) { + ImageViewerDialog.getInstance().destroy(); + } } Index: /trunk/test/unit/org/openstreetmap/josm/gui/layer/markerlayer/ImageMarkerTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/layer/markerlayer/ImageMarkerTest.java (revision 18894) +++ /trunk/test/unit/org/openstreetmap/josm/gui/layer/markerlayer/ImageMarkerTest.java (revision 18895) @@ -8,4 +8,5 @@ import java.net.MalformedURLException; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.openstreetmap.josm.TestUtils; @@ -13,4 +14,5 @@ import org.openstreetmap.josm.data.gpx.GpxData; import org.openstreetmap.josm.data.gpx.WayPoint; +import org.openstreetmap.josm.gui.layer.geoimage.ImageViewerDialog; import org.openstreetmap.josm.testutils.annotations.BasicPreferences; import org.openstreetmap.josm.testutils.annotations.Main; @@ -22,4 +24,11 @@ @Main class ImageMarkerTest { + @AfterEach + void tearDown() { + if (ImageViewerDialog.hasInstance()) { + ImageViewerDialog.getInstance().destroy(); + } + } + /** * Unit test of {@link ImageMarker#ImageMarker}.