Changeset 18283 in josm for trunk/src/org

Timestamp:

2021-10-17T15:17:42+02:00 (3 years ago)

Author:

Don-vip

Message:

fix #21427 - further simplify UploadDialog (patch by marcello, modified)

• The dialog was simplified by combining the function of two radiobuttons and one combobox into one combobox.
• When an open changeset was selected on tab 2, existing tags on the open changeset could overwrite the data the user entered on tab 1. The user might spot this by looking closely at the tag table on tab 2, but then he may not. This non-obvious behaviour was removed.
• The exception thrown when closing an already closed changeset was fixed.
• More cosmetic changes to the dialog.
• Maybe also a solution to #19319, #21387 (added revalidate()).

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/UploadAction.java (modified) (3 diffs)
• data/osm/Changeset.java (modified) (1 diff)
• data/osm/ChangesetCache.java (modified) (2 diffs)
• gui/io/BasicUploadSettingsPanel.java (modified) (8 diffs)
• gui/io/ChangesetCellRenderer.java (modified) (1 diff)
• gui/io/ChangesetManagementPanel.java (modified) (8 diffs)
• gui/io/OpenChangesetComboBoxModel.java (deleted)
• gui/io/UploadDialog.java (modified) (17 diffs)
• gui/io/UploadDialogModel.java (modified) (4 diffs)
• gui/io/UploadParameterSummaryPanel.java (modified) (2 diffs)
• gui/io/UploadPrimitivesTask.java (modified) (6 diffs)
• gui/io/UploadStrategySelectionPanel.java (modified) (11 diffs)
• gui/io/UploadedObjectsSummaryPanel.java (modified) (2 diffs)
• gui/tagging/TagEditorModel.java (modified) (8 diffs)
• gui/tagging/TagTable.java (modified) (3 diffs)
• gui/widgets/JosmComboBoxModel.java (modified) (7 diffs)
• io/ChangesetClosedException.java (modified) (3 diffs)
• io/OsmApi.java (modified) (5 diffs)
• io/UploadStrategySpecification.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/actions/UploadAction.java

@@ -36 +36 @@
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.ImageProvider;
+import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Shortcut;
 import org.openstreetmap.josm.tools.Utils;
@@ -240 +241 @@
 
         final UploadDialog dialog = UploadDialog.getUploadDialog();
+        dialog.setUploadedPrimitives(apiData);
         dialog.initLifeCycle(layer.getDataSet());
-        dialog.setUploadedPrimitives(apiData);
         dialog.setVisible(true);
         dialog.rememberUserInput();
@@ -267 +268 @@
 
         UploadStrategySpecification uploadStrategySpecification = dialog.getUploadStrategySpecification();
+        Logging.info("Starting upload with tags {0}", changesetTags);
+        Logging.info(uploadStrategySpecification.toString());
+        Logging.info(cs.toString());
         dialog.clean();
 

trunk/src/org/openstreetmap/josm/data/osm/Changeset.java

@@ -318 +318 @@
                 throw new IllegalArgumentException("Changeset tag value is too long: "+value);
         });
-        this.tags = keys;
+        this.tags = new HashMap<>(keys);
     }
 

trunk/src/org/openstreetmap/josm/data/osm/ChangesetCache.java

@@ -13 +13 @@
 
 import org.openstreetmap.josm.data.UserIdentityManager;
+import org.openstreetmap.josm.io.ChangesetQuery;
+import org.openstreetmap.josm.io.OsmServerChangesetReader;
+import org.openstreetmap.josm.io.OsmTransferException;
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.spi.preferences.PreferenceChangeEvent;
 import org.openstreetmap.josm.spi.preferences.PreferenceChangedListener;
+import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.SubclassFilteredCollection;
 import org.openstreetmap.josm.tools.Utils;
@@ -255 +259 @@
     }
 
+    /**
+     * Refreshes the changesets from the server.
+     * <p>
+     * The server automatically closes changesets after a timeout.  We don't get notified of this
+     * fact when it happens.  This method requests a fresh list from the server and updates the
+     * local list.  Calling this method reduces (but does not eliminate) the probability of
+     * attempting an upload to an already closed changeset.
+     *
+     * @throws OsmTransferException on server error
+     */
+    public void refreshChangesetsFromServer() throws OsmTransferException {
+        OsmServerChangesetReader reader;
+        synchronized (this) {
+            reader = new OsmServerChangesetReader();
+        }
+        List<Changeset> server = reader.queryChangesets(ChangesetQuery.forCurrentUser().beingOpen(true), null);
+        Logging.info("{0} open changesets on server", server.size());
+
+        DefaultChangesetCacheEvent e = new DefaultChangesetCacheEvent(this);
+        // flag timed out changesets
+        for (Changeset cs : getOpenChangesetsForCurrentUser()) {
+            if (!server.contains(cs))
+                remove(cs.getId(), e);
+        }
+        for (Changeset cs: server) {
+            update(cs, e);
+        }
+        fireChangesetCacheEvent(e);
+    }
+
     /* ------------------------------------------------------------------------- */
     /* interface PreferenceChangedListener                                       */

trunk/src/org/openstreetmap/josm/gui/io/BasicUploadSettingsPanel.java

@@ -103 +103 @@
     }
 
+    protected void build() {
+        setLayout(new GridBagLayout());
+        setBorder(BorderFactory.createEmptyBorder(3, 3, 3, 3));
+        GBC gbc = GBC.eop().fill(GBC.HORIZONTAL);
+        add(buildUploadCommentPanel(), gbc);
+        add(buildUploadSourcePanel(), gbc);
+        add(pnlUploadParameterSummary, gbc);
+        if (Config.getPref().getBoolean("upload.show.review.request", true)) {
+            add(cbRequestReview, gbc);
+            cbRequestReview.addItemListener(this);
+        }
+        add(areaValidatorFeedback, gbc);
+        add(new JPanel(), GBC.std().fill(GBC.BOTH));
+    }
+
     protected JPanel buildUploadCommentPanel() {
         JPanel pnl = new JPanel(new GridBagLayout());
@@ -114 +129 @@
         editor.addFocusListener(this);
         editor.addActionListener(this);
-        pnl.add(hcbUploadComment, GBC.eol().fill(GBC.HORIZONTAL));
-        pnl.add(uploadCommentFeedback, GBC.eol().insets(0, 3, 0, 0).fill(GBC.HORIZONTAL));
+        GBC gbc = GBC.eol().insets(3).fill(GBC.HORIZONTAL);
+        pnl.add(hcbUploadComment, gbc);
+        pnl.add(uploadCommentFeedback, gbc);
         return pnl;
     }
@@ -142 +158 @@
         obtainSource.add(obtainSourceOnce, GBC.std().anchor(GBC.WEST));
         obtainSource.add(new JLabel(), GBC.eol().fill(GBC.HORIZONTAL));
-        if (Config.getPref().getBoolean("upload.show.automatic.source", true)) {
-            pnl.add(obtainSource, GBC.eol().insets(0, 0, 10, 3).fill(GBC.HORIZONTAL));
-        }
 
         hcbUploadSource.setToolTipText(tr("Enter a source"));
@@ -153 +166 @@
         editor.addFocusListener(this);
         editor.addActionListener(this);
-        pnl.add(hcbUploadSource, GBC.eol().fill(GBC.HORIZONTAL));
-        pnl.add(hcbUploadSourceFeedback, GBC.eol().insets(0, 3, 0, 0).fill(GBC.HORIZONTAL));
-
+        GBC gbc = GBC.eol().insets(3).fill(GBC.HORIZONTAL);
+        if (Config.getPref().getBoolean("upload.show.automatic.source", true)) {
+            pnl.add(obtainSource, gbc);
+        }
+        pnl.add(hcbUploadSource, gbc);
+        pnl.add(hcbUploadSourceFeedback, gbc);
         return pnl;
     }
@@ -162 +178 @@
      * Initializes this life cycle of the panel.
      *
-     * Adds any changeset tags to the map.
+     * Adds the comment and source tags from history, and/or obtains the source from the layer if
+     * the user said so.
      *
      * @param map Map where tags are added to.
@@ -179 +196 @@
         hcbUploadSource.getModel().prefs().load(SOURCE_HISTORY_KEY, getDefaultSources());
         hcbUploadSource.discardAllUndoableEdits();
+        hcbUploadComment.getEditorComponent().requestFocusInWindow();
+        uploadCommentValidator.validate();
+        uploadSourceValidator.validate();
     }
 
@@ -233 +253 @@
     protected List<UploadTextComponentValidator> getUploadTextValidators() {
         return Arrays.asList(areaValidator, uploadCommentValidator, uploadSourceValidator);
-    }
-
-    protected void build() {
-        setLayout(new GridBagLayout());
-        setBorder(BorderFactory.createEmptyBorder(3, 3, 3, 3));
-        GBC gbc = GBC.eol().insets(0, 0, 0, 20).fill(GBC.HORIZONTAL);
-        add(buildUploadCommentPanel(), gbc);
-        add(buildUploadSourcePanel(), gbc);
-        add(pnlUploadParameterSummary, gbc);
-        if (Config.getPref().getBoolean("upload.show.review.request", true)) {
-            add(cbRequestReview, gbc);
-            cbRequestReview.addItemListener(this);
-        }
-        add(areaValidatorFeedback, gbc);
-        add(new JPanel(), GBC.std().fill(GBC.BOTH));
     }
 
@@ -266 +271 @@
         // store current value of obtaining source automatically
         Config.getPref().putBoolean("upload.source.obtainautomatically", obtainSourceAutomatically.isSelected());
-    }
-
-    /**
-     * Initializes the panel for user input
-     */
-    public void startUserInput() {
-        hcbUploadComment.getEditorComponent().requestFocusInWindow();
-        uploadCommentValidator.validate();
-        uploadSourceValidator.validate();
     }
 

trunk/src/org/openstreetmap/josm/gui/io/ChangesetCellRenderer.java

@@ -63 +63 @@
         if (cs != null) {
             setIcon(icon);
-            StringBuilder sb = new StringBuilder();
-            String comment = cs.getComment();
-            if (!comment.isEmpty()) {
-                sb.append(cs.getId()).append(" - ").append(comment);
-            } else if (cs.get("name") != null) {
-                sb.append(cs.getId()).append(" - ").append(cs.get("name"));
+            if (cs.getId() == 0) {
+                setText("New changeset");
             } else {
-                sb.append(tr("Changeset {0}", cs.getId()));
+                StringBuilder sb = new StringBuilder();
+                String comment = cs.getComment();
+                if (!comment.isEmpty()) {
+                    sb.append(cs.getId()).append(" - ").append(comment);
+                } else if (cs.get("name") != null) {
+                    sb.append(cs.getId()).append(" - ").append(cs.get("name"));
+                } else {
+                    sb.append(tr("Changeset {0}", cs.getId()));
+                }
+                setText(sb.toString());
             }
-            setText(sb.toString());
             setToolTipText(buildToolTipText(cs));
         } else {
-            setText(tr("No open changesets"));
+            setIcon(null);
+            setText("");
         }
         return this;

trunk/src/org/openstreetmap/josm/gui/io/ChangesetManagementPanel.java

@@ -12 +12 @@
 import java.awt.event.ItemListener;
 import java.util.Collections;
+import java.util.Optional;
 
 import javax.swing.AbstractAction;
 import javax.swing.BorderFactory;
-import javax.swing.ButtonGroup;
 import javax.swing.JButton;
 import javax.swing.JCheckBox;
 import javax.swing.JPanel;
-import javax.swing.JRadioButton;
-import javax.swing.event.ListDataEvent;
-import javax.swing.event.ListDataListener;
+import javax.swing.SwingUtilities;
 
 import org.openstreetmap.josm.data.osm.Changeset;
 import org.openstreetmap.josm.data.osm.ChangesetCache;
+import org.openstreetmap.josm.data.osm.ChangesetCacheEvent;
+import org.openstreetmap.josm.data.osm.ChangesetCacheListener;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.widgets.JMultilineLabel;
 import org.openstreetmap.josm.gui.widgets.JosmComboBox;
+import org.openstreetmap.josm.gui.widgets.JosmComboBoxModel;
+import org.openstreetmap.josm.io.OsmTransferException;
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.ImageProvider;
@@ -45 +47 @@
  * </ul>
  */
-public class ChangesetManagementPanel extends JPanel implements ListDataListener {
+public class ChangesetManagementPanel extends JPanel implements ItemListener, ChangesetCacheListener {
     static final String SELECTED_CHANGESET_PROP = ChangesetManagementPanel.class.getName() + ".selectedChangeset";
     static final String CLOSE_CHANGESET_AFTER_UPLOAD = ChangesetManagementPanel.class.getName() + ".closeChangesetAfterUpload";
 
-    private JRadioButton rbUseNew;
-    private JRadioButton rbExisting;
     private JosmComboBox<Changeset> cbOpenChangesets;
+    private JosmComboBoxModel<Changeset> model;
     private JCheckBox cbCloseAfterUpload;
-    private OpenChangesetComboBoxModel model;
-
-    /** the changeset comment model */
-    private final transient UploadDialogModel uploadDialogModel;
+    private JButton btnClose;
 
     /**
      * Constructs a new {@code ChangesetManagementPanel}.
      *
-     * @param uploadDialogModel The tag editor model.
-     *
-     * @since 18173 (signature)
-     */
-    public ChangesetManagementPanel(UploadDialogModel uploadDialogModel) {
-        this.uploadDialogModel = uploadDialogModel;
+     * @since 18283 (signature)
+     */
+    public ChangesetManagementPanel() {
         build();
-        refreshGUI();
+    }
+
+    /**
+     * Initializes this life cycle of the panel.
+     *
+     * @since 18283
+     */
+    public void initLifeCycle() {
+        refreshChangesets();
+    }
+
+    /**
+     * Returns the model in use.
+     * @return the model
+     */
+    public JosmComboBoxModel<Changeset> getModel() {
+        return model;
     }
 
@@ -76 +87 @@
     protected void build() {
         setLayout(new GridBagLayout());
+        setBorder(BorderFactory.createTitledBorder(tr("Please select a changeset:")));
+
         GridBagConstraints gc = new GridBagConstraints();
-        setBorder(BorderFactory.createEmptyBorder(3, 3, 3, 3));
-
-        ButtonGroup bgUseNewOrExisting = new ButtonGroup();
-
-        gc.gridwidth = 4;
         gc.gridx = 0;
         gc.gridy = 0;
-        gc.fill = GridBagConstraints.HORIZONTAL;
         gc.weightx = 1.0;
         gc.weighty = 0.0;
-        gc.insets = new Insets(0, 0, 5, 0);
-        add(new JMultilineLabel(
-                tr("Please decide what changeset the data is uploaded to and whether to close the changeset after the next upload.")), gc);
-
-        gc.gridwidth = 4;
-        gc.gridy = 1;
         gc.fill = GridBagConstraints.HORIZONTAL;
-        gc.weightx = 1.0;
-        gc.weighty = 0.0;
-        gc.insets = new Insets(0, 0, 0, 0);
-        gc.anchor = GridBagConstraints.FIRST_LINE_START;
-        rbUseNew = new JRadioButton(tr("Upload to a new changeset"));
-        rbUseNew.setToolTipText(tr("Open a new changeset and use it in the next upload"));
-        bgUseNewOrExisting.add(rbUseNew);
-        add(rbUseNew, gc);
-
-        gc.gridx = 0;
-        gc.gridy = 2;
+        gc.anchor = GridBagConstraints.NORTHWEST;
+        gc.insets = new Insets(3, 3, 3, 3);
+
+        gc.gridwidth = 3;
+        add(new JMultilineLabel(tr(
+            "Please select which changeset the data shall be uploaded to and whether to close that changeset after the next upload."
+            )), gc);
+
         gc.gridwidth = 1;
-        gc.weightx = 0.0;
-        gc.fill = GridBagConstraints.HORIZONTAL;
-        rbExisting = new JRadioButton(tr("Upload to an existing changeset"));
-        rbExisting.setToolTipText(tr("Upload data to an already existing and open changeset"));
-        bgUseNewOrExisting.add(rbExisting);
-        add(rbExisting, gc);
-
-        gc.gridx = 1;
-        gc.gridy = 2;
-        gc.gridwidth = 1;
-        gc.weightx = 1.0;
-        model = new OpenChangesetComboBoxModel();
-        ChangesetCache.getInstance().addChangesetCacheListener(model);
+        gc.gridy++;
+        model = new JosmComboBoxModel<>();
         cbOpenChangesets = new JosmComboBox<>(model);
-        cbOpenChangesets.setToolTipText(tr("Select an open changeset"));
+        cbOpenChangesets.setToolTipText(tr("Select a changeset"));
         cbOpenChangesets.setRenderer(new ChangesetCellRenderer());
-        cbOpenChangesets.addItemListener(new ChangesetListItemStateListener());
         Dimension d = cbOpenChangesets.getPreferredSize();
         d.width = 200;
@@ -128 +114 @@
         d.width = 100;
         cbOpenChangesets.setMinimumSize(d);
-        model.addListDataListener(this);
         add(cbOpenChangesets, gc);
-
-        gc.gridx = 2;
-        gc.gridy = 2;
-        gc.weightx = 0.0;
-        gc.gridwidth = 1;
+        int h = cbOpenChangesets.getPreferredSize().height;
+        Dimension prefSize = new Dimension(h, h);
+
+        gc.gridx++;
         gc.weightx = 0.0;
         JButton btnRefresh = new JButton(new RefreshAction());
-        btnRefresh.setMargin(new Insets(0, 0, 0, 0));
+        btnRefresh.setPreferredSize(prefSize);
+        btnRefresh.setMinimumSize(prefSize);
         add(btnRefresh, gc);
 
-        gc.gridx = 3;
-        gc.gridy = 2;
-        gc.gridwidth = 1;
+        gc.gridx++;
         CloseChangesetAction closeChangesetAction = new CloseChangesetAction();
-        JButton btnClose = new JButton(closeChangesetAction);
-        btnClose.setMargin(new Insets(0, 0, 0, 0));
-        cbOpenChangesets.addItemListener(closeChangesetAction);
-        rbExisting.addItemListener(closeChangesetAction);
+        btnClose = new JButton(closeChangesetAction);
+        btnClose.setPreferredSize(prefSize);
+        btnClose.setMinimumSize(prefSize);
         add(btnClose, gc);
 
+        gc.gridy++;
         gc.gridx = 0;
-        gc.gridy = 3;
-        gc.gridwidth = 4;
+        gc.gridwidth = 3;
         gc.weightx = 1.0;
         cbCloseAfterUpload = new JCheckBox(tr("Close changeset after upload"));
         cbCloseAfterUpload.setToolTipText(tr("Select to close the changeset after the next upload"));
         add(cbCloseAfterUpload, gc);
+
+        cbOpenChangesets.addItemListener(this);
+        cbOpenChangesets.addItemListener(closeChangesetAction);
+
         cbCloseAfterUpload.setSelected(Config.getPref().getBoolean("upload.changeset.close", true));
         cbCloseAfterUpload.addItemListener(new CloseAfterUploadItemStateListener());
 
-        rbUseNew.getModel().addItemListener(new RadioButtonHandler());
-        rbExisting.getModel().addItemListener(new RadioButtonHandler());
-    }
-
-    protected void refreshGUI() {
-        rbExisting.setEnabled(model.getSize() > 0);
-        if (model.getSize() == 0 && !rbUseNew.isSelected()) {
-            rbUseNew.setSelected(true);
-        }
-        cbOpenChangesets.setEnabled(model.getSize() > 0 && rbExisting.isSelected());
+        ChangesetCache.getInstance().addChangesetCacheListener(this);
     }
 
     /**
      * Sets the changeset to be used in the next upload
+     * <p>
+     * Note: The changeset may be a new changeset that was automatically opened because the old
+     * changeset overflowed.  In that case it was already added to the changeset cache and the
+     * combobox.
      *
      * @param cs the changeset
+     * @see UploadPrimitivesTask#handleChangesetFullResponse
      */
     public void setSelectedChangesetForNextUpload(Changeset cs) {
-        int idx = model.getIndexOf(cs);
-        if (idx >= 0) {
-            rbExisting.setSelected(true);
-            model.setSelectedItem(cs);
-        }
-    }
-
-    /**
-     * Replies the currently selected changeset. null, if no changeset is
-     * selected or if the user has chosen to use a new changeset.
-     *
-     * @return the currently selected changeset. null, if no changeset is
-     * selected.
+        model.setSelectedItem(cs);
+    }
+
+    /**
+     * Returns the currently selected changeset or an empty new one.
+     *
+     * @return the currently selected changeset
      */
     public Changeset getSelectedChangeset() {
-        if (rbUseNew.isSelected())
-            return null;
-        return (Changeset) cbOpenChangesets.getSelectedItem();
+        return Optional.ofNullable((Changeset) model.getSelectedItem()).orElse(new Changeset());
     }
 
@@ -206 +180 @@
     }
 
-    /* ---------------------------------------------------------------------------- */
-    /* Interface ListDataListener                                                   */
-    /* ---------------------------------------------------------------------------- */
+    /**
+     * Listens to changes in the selected changeset and fires property change events.
+     */
     @Override
-    public void contentsChanged(ListDataEvent e) {
-        refreshGUI();
-    }
-
-    @Override
-    public void intervalAdded(ListDataEvent e) {
-        refreshGUI();
-    }
-
-    @Override
-    public void intervalRemoved(ListDataEvent e) {
-        refreshGUI();
-    }
-
-    /**
-     * Listens to changes in the selected changeset and fires property change events.
-     */
-    class ChangesetListItemStateListener implements ItemListener {
-        @Override
-        public void itemStateChanged(ItemEvent e) {
-            Changeset cs = (Changeset) cbOpenChangesets.getSelectedItem();
-            if (cs == null) return;
-            if (rbExisting.isSelected()) {
-                firePropertyChange(SELECTED_CHANGESET_PROP, null, cs);
-            }
-        }
+    public void itemStateChanged(ItemEvent e) {
+        firePropertyChange(SELECTED_CHANGESET_PROP, null, model.getSelectedItem());
     }
 
@@ -261 +211 @@
 
     /**
-     * Listens to changes in the two radio buttons rbUseNew and rbUseExisting.
-     */
-    class RadioButtonHandler implements ItemListener {
-        @Override
-        public void itemStateChanged(ItemEvent e) {
-            if (rbUseNew.isSelected()) {
-                cbOpenChangesets.setEnabled(false);
-                firePropertyChange(SELECTED_CHANGESET_PROP, null, null);
-            } else if (rbExisting.isSelected()) {
-                cbOpenChangesets.setEnabled(true);
-                if (cbOpenChangesets.getSelectedItem() == null) {
-                    model.selectFirstChangeset();
-                }
-                Changeset cs = (Changeset) cbOpenChangesets.getSelectedItem();
-                if (cs == null) return;
-                uploadDialogModel.putAll(cs.getKeys());
-                firePropertyChange(SELECTED_CHANGESET_PROP, null, cs);
-            }
-        }
-    }
-
-    /**
      * Refreshes the list of open changesets
      */
@@ -315 +243 @@
 
         protected void refreshEnabledState() {
-            setEnabled(
-                    cbOpenChangesets.getModel().getSize() > 0
-                    && cbOpenChangesets.getSelectedItem() != null
-                    && rbExisting.isSelected()
-            );
+            setEnabled(!getSelectedChangeset().isNew());
         }
 
@@ -327 +251 @@
         }
     }
+
+    /**
+     * Refreshes the changesets combobox form the server.
+     * <p>
+     * Note: This calls into {@link #refreshCombo} through {@link #changesetCacheUpdated}
+     *
+     * @see ChangesetCache#refreshChangesetsFromServer
+     */
+    protected void refreshChangesets() {
+        try {
+            ChangesetCache.getInstance().refreshChangesetsFromServer();
+        } catch (OsmTransferException e) {
+            return;
+        }
+    }
+
+    private void refreshCombo() {
+        Changeset selected = (Changeset) cbOpenChangesets.getSelectedItem();
+        model.removeAllElements();
+        model.addElement(new Changeset());
+        model.addAllElements(ChangesetCache.getInstance().getOpenChangesetsForCurrentUser());
+        cbOpenChangesets.setSelectedItem(selected != null && model.getIndexOf(selected) != -1 ? selected : model.getElementAt(0));
+    }
+
+    @Override
+    public void changesetCacheUpdated(ChangesetCacheEvent event) {
+        // This listener might have been called by a background task.
+        SwingUtilities.invokeLater(() -> refreshCombo());
+    }
 }

trunk/src/org/openstreetmap/josm/gui/io/UploadDialog.java

@@ -10 +10 @@
 import java.awt.Dimension;
 import java.awt.FlowLayout;
+import java.awt.GridBagConstraints;
 import java.awt.GridBagLayout;
 import java.awt.event.ActionEvent;
@@ -24 +25 @@
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Optional;
 import java.util.stream.Collectors;
 
@@ -34 +34 @@
 import javax.swing.JSplitPane;
 import javax.swing.JTabbedPane;
-import javax.swing.border.TitledBorder;
 
 import org.openstreetmap.josm.data.APIDataSet;
@@ -58 +57 @@
 import org.openstreetmap.josm.tools.ImageProvider;
 import org.openstreetmap.josm.tools.InputMapUtils;
-import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Utils;
 
@@ -66 +64 @@
  * @since 2025
  */
-public class UploadDialog extends AbstractUploadDialog implements PropertyChangeListener, PreferenceChangedListener {
+public class UploadDialog extends AbstractUploadDialog implements PreferenceChangedListener, PropertyChangeListener {
     /** the unique instance of the upload dialog */
     private static UploadDialog uploadDialog;
@@ -72 +70 @@
     /** the panel with the objects to upload */
     private UploadedObjectsSummaryPanel pnlUploadedObjects;
+
+    /** the "description" tab */
+    private BasicUploadSettingsPanel pnlBasicUploadSettings;
+
     /** the panel to select the changeset used */
     private ChangesetManagementPanel pnlChangesetManagement;
-
-    private BasicUploadSettingsPanel pnlBasicUploadSettings;
-
+    /** the panel to select the upload strategy */
     private UploadStrategySelectionPanel pnlUploadStrategySelectionPanel;
 
-    private TitledBorder tagSettingsBorder;
-    /** a border around the tag editor panel */
-    private JPanel pnlTagEditorBorder;
     /** the tag editor panel */
     private TagEditorPanel pnlTagEditor;
@@ -97 +94 @@
      * Constructs a new {@code UploadDialog}.
      */
-    public UploadDialog() {
+    protected UploadDialog() {
         super(GuiHelper.getFrameForComponent(MainApplication.getMainFrame()), ModalityType.DOCUMENT_MODAL);
         build();
@@ -139 +136 @@
 
         JPanel pnlSettings = new JPanel(new GridBagLayout());
-        pnlTagEditorBorder = new JPanel(new BorderLayout());
-        tagSettingsBorder = BorderFactory.createTitledBorder(tr("Tags of new changeset"));
-        pnlTagEditorBorder.setBorder(tagSettingsBorder);
+        pnlSettings.setBorder(BorderFactory.createEmptyBorder(3, 3, 3, 3));
+        JPanel pnlTagEditorBorder = new JPanel(new BorderLayout());
+        pnlTagEditorBorder.setBorder(BorderFactory.createTitledBorder(tr("Changeset tags:")));
         pnlTagEditor = new TagEditorPanel(model, null, Changeset.MAX_CHANGESET_TAG_LENGTH);
         pnlTagEditorBorder.add(pnlTagEditor, BorderLayout.CENTER);
 
-        pnlChangesetManagement = new ChangesetManagementPanel(model);
+        pnlChangesetManagement = new ChangesetManagementPanel();
         pnlUploadStrategySelectionPanel = new UploadStrategySelectionPanel();
-        pnlSettings.add(pnlChangesetManagement, GBC.eop().fill(GBC.HORIZONTAL));
-        pnlSettings.add(pnlUploadStrategySelectionPanel, GBC.eop().fill(GBC.HORIZONTAL));
-        pnlSettings.add(pnlTagEditorBorder, GBC.eol().fill(GBC.BOTH));
+        pnlSettings.add(pnlChangesetManagement, GBC.eop().fill(GridBagConstraints.HORIZONTAL));
+        pnlSettings.add(pnlUploadStrategySelectionPanel, GBC.eop().fill(GridBagConstraints.HORIZONTAL));
+        pnlSettings.add(pnlTagEditorBorder, GBC.eol().fill(GridBagConstraints.BOTH));
 
         tpConfigPanels.add(pnlSettings);
@@ -197 +194 @@
         addWindowListener(new WindowEventHandler());
 
-        // make sure the configuration panels listen to each other changes
+        // make sure the configuration panels listen to each others changes
         //
+        UploadParameterSummaryPanel sp = pnlBasicUploadSettings.getUploadParameterSummaryPanel();
+        // the summary panel must know everything
+        pnlChangesetManagement.addPropertyChangeListener(sp);
+        pnlUploadedObjects.addPropertyChangeListener(sp);
+        pnlUploadStrategySelectionPanel.addPropertyChangeListener(sp);
+
+        // update tags from selected changeset
         pnlChangesetManagement.addPropertyChangeListener(this);
-        pnlChangesetManagement.addPropertyChangeListener(
-                pnlBasicUploadSettings.getUploadParameterSummaryPanel()
-        );
-        pnlUploadedObjects.addPropertyChangeListener(pnlUploadStrategySelectionPanel);
-        pnlUploadedObjects.addPropertyChangeListener(
-                pnlBasicUploadSettings.getUploadParameterSummaryPanel()
-        );
-        pnlUploadStrategySelectionPanel.addPropertyChangeListener(this);
-        pnlUploadStrategySelectionPanel.addPropertyChangeListener(
-                pnlBasicUploadSettings.getUploadParameterSummaryPanel()
-        );
 
         // users can click on either of two links in the upload parameter
@@ -242 +235 @@
         this.dataSet = dataSet;
         pnlBasicUploadSettings.initLifeCycle(map);
+        pnlChangesetManagement.initLifeCycle();
         model.clear();
-        model.putAll(map);
-        model.putAll(this.dataSet);
+        model.putAll(map);          // init with tags from history
+        model.putAll(this.dataSet); // overwrite with tags from the dataset
+
+        tpConfigPanels.setSelectedIndex(0);
+        pnlTagEditor.initAutoCompletion(MainApplication.getLayerManager().getEditLayer());
+        pnlUploadStrategySelectionPanel.initFromPreferences();
+
+        // update the summary
+        UploadParameterSummaryPanel sumPnl = pnlBasicUploadSettings.getUploadParameterSummaryPanel();
+        sumPnl.setUploadStrategySpecification(pnlUploadStrategySelectionPanel.getUploadStrategySpecification());
+        sumPnl.setCloseChangesetAfterNextUpload(pnlChangesetManagement.isCloseChangesetAfterUpload());
     }
 
@@ -255 +258 @@
      */
     public void setUploadedPrimitives(APIDataSet toUpload) {
+        UploadParameterSummaryPanel sumPnl = pnlBasicUploadSettings.getUploadParameterSummaryPanel();
         if (toUpload == null) {
             if (pnlUploadedObjects != null) {
                 List<OsmPrimitive> emptyList = Collections.emptyList();
                 pnlUploadedObjects.setUploadedPrimitives(emptyList, emptyList, emptyList);
+                sumPnl.setNumObjects(0);
             }
             return;
         }
-        pnlBasicUploadSettings.setUploadedPrimitives(toUpload.getPrimitives());
+        List<OsmPrimitive> l = toUpload.getPrimitives();
+        pnlBasicUploadSettings.setUploadedPrimitives(l);
         pnlUploadedObjects.setUploadedPrimitives(
                 toUpload.getPrimitivesToAdd(),
@@ -268 +274 @@
                 toUpload.getPrimitivesToDelete()
         );
+        sumPnl.setNumObjects(l.size());
+        pnlUploadStrategySelectionPanel.setNumUploadedObjects(l.size());
     }
 
@@ -285 +293 @@
 
     /**
-     * Initializes the panel for user input
-     */
-    public void startUserInput() {
-        tpConfigPanels.setSelectedIndex(0);
-        pnlBasicUploadSettings.startUserInput();
-        pnlTagEditor.initAutoCompletion(MainApplication.getLayerManager().getEditLayer());
-        pnlUploadStrategySelectionPanel.initFromPreferences();
-        UploadParameterSummaryPanel pnl = pnlBasicUploadSettings.getUploadParameterSummaryPanel();
-        pnl.setUploadStrategySpecification(pnlUploadStrategySelectionPanel.getUploadStrategySpecification());
-        pnl.setCloseChangesetAfterNextUpload(pnlChangesetManagement.isCloseChangesetAfterUpload());
-        pnl.setNumObjects(pnlUploadedObjects.getNumObjectsToUpload());
-    }
-
-    /**
-     * Replies the current changeset
-     *
-     * @return the current changeset
+     * Returns the changeset to use complete with tags
+     *
+     * @return the changeset to use
      */
     public Changeset getChangeset() {
-        Changeset cs = Optional.ofNullable(pnlChangesetManagement.getSelectedChangeset()).orElseGet(Changeset::new);
-        cs.setKeys(model.getTags(false));
+        Changeset cs = pnlChangesetManagement.getSelectedChangeset();
+        cs.setKeys(getTags(true));
         return cs;
     }
@@ -337 +331 @@
     @Override
     public String getUploadComment() {
-        return model.getValue("comment");
+        return model.getValue(UploadDialogModel.COMMENT);
     }
 
     @Override
     public String getUploadSource() {
-        return model.getValue("source");
+        return model.getValue(UploadDialogModel.SOURCE);
     }
 
@@ -355 +349 @@
                     )
             ).applySafe(this);
-            startUserInput();
         } else if (isShowing()) { // Avoid IllegalComponentStateException like in #8775
             new WindowGeometry(this).remember(getClass().getName() + ".geometry");
@@ -437 +430 @@
         public void actionPerformed(ActionEvent e) {
             Map<String, String> tags = dialog.getTags(true);
-            Logging.info("Starting upload with tags {0}", tags);
-
-            /* test for empty tags in the changeset metadata and proceed only after user's confirmation.
-             * though, accept if key and value are empty (cf. xor). */
+
+            // If there are empty tags in the changeset proceed only after user's confirmation.
             List<String> emptyChangesetTags = new ArrayList<>();
             for (final Entry<String, String> i : tags.entrySet()) {
                 final boolean isKeyEmpty = Utils.isStripEmpty(i.getKey());
                 final boolean isValueEmpty = Utils.isStripEmpty(i.getValue());
-                final boolean ignoreKey = "comment".equals(i.getKey()) || "source".equals(i.getKey());
-                if ((isKeyEmpty ^ isValueEmpty) && !ignoreKey) {
+                final boolean ignoreKey = UploadDialogModel.isCommentOrSource(i.getKey());
+                if ((isKeyEmpty || isValueEmpty) && !ignoreKey) {
                     emptyChangesetTags.add(tr("{0}={1}", i.getKey(), i.getValue()));
                 }
@@ -528 +519 @@
     public void propertyChange(PropertyChangeEvent evt) {
         if (evt.getPropertyName().equals(ChangesetManagementPanel.SELECTED_CHANGESET_PROP)) {
+            // put the tags from the newly selected changeset into the model
             Changeset cs = (Changeset) evt.getNewValue();
-            if (cs == null) {
-                tagSettingsBorder.setTitle(tr("Tags of new changeset"));
-            } else {
-                tagSettingsBorder.setTitle(tr("Tags of changeset {0}", cs.getId()));
+            if (cs != null) {
+                for (Map.Entry<String, String> entry : cs.getKeys().entrySet()) {
+                    String key = entry.getKey();
+                    // do NOT overwrite comment and source when selecting a changeset, it is confusing
+                    if (!UploadDialogModel.isCommentOrSource(key))
+                        model.put(key, entry.getValue());
+                }
             }
         }

trunk/src/org/openstreetmap/josm/gui/io/UploadDialogModel.java

@@ -24 +24 @@
     /** the "created_by" changeset OSM key */
     private static final String CREATED_BY = "created_by";
+    /** the "comment" changeset OSM key */
+    public static final String COMMENT = "comment";
+    /** the "source" changeset OSM key */
+    public static final String SOURCE = "source";
     /** the user-agent */
     private final String agent = Version.getInstance().getAgentString(false);
@@ -39 +43 @@
                 // add "hashtags" if any
                 if (hashtags) {
-                    put("hashtags", findHashTags(getValue("comment")));
+                    put("hashtags", findHashTags(getValue(COMMENT)));
                 }
                 // add/update "created_by"
@@ -116 +120 @@
         if (!l.isEmpty()) {
             if (value != null)
-                l.get(0).setValue(value);
+                for (TagModel tm : l) {
+                    tm.setValue(value);
+                }
             else
-                tags.remove(l.get(0));
+                tags.removeIf(tm -> tm.getName().equals(key));
         } else if (value != null) {
             tags.add(new TagModel(key, value));
@@ -163 +169 @@
         if (dataSet != null) {
             putAll(dataSet.getChangeSetTags());
-            put("comment", addHashTagsFromDataSet(getValue("comment"), dataSet));
+            put(COMMENT, addHashTagsFromDataSet(getValue(COMMENT), dataSet));
         }
     }
+
+    /**
+     * Determines if the key is "comment" or "source".
+     * @param key changeset key
+     * @return {@code true} if the key is "comment" or "source"
+     * @since 18283
+     */
+    public static boolean isCommentOrSource(String key) {
+        return COMMENT.equals(key) || SOURCE.equals(key);
+    }
 }

trunk/src/org/openstreetmap/josm/gui/io/UploadParameterSummaryPanel.java

@@ -54 +54 @@
             return tr("Objects are uploaded to a <strong>new changeset</strong>.");
         } else {
-            return tr("Objects are uploaded to the <strong>open changeset</strong> {0} with upload comment ''{1}''.",
+            return tr("Objects are uploaded to the <strong>open changeset</strong> {0} ''{1}''.",
                     selectedChangeset.getId(),
                     selectedChangeset.getComment()
@@ -200 +200 @@
             closeChangesetAfterNextUpload = (Boolean) evt.getNewValue();
             updateSummary();
-        } else if (evt.getPropertyName().equals(UploadedObjectsSummaryPanel.NUM_OBJECTS_TO_UPLOAD_PROP)) {
-            numObjects = (Integer) evt.getNewValue();
-            updateSummary();
         } else if (evt.getPropertyName().equals(UploadStrategySelectionPanel.UPLOAD_STRATEGY_SPECIFICATION_PROP)) {
             this.spec = (UploadStrategySpecification) evt.getNewValue();

trunk/src/org/openstreetmap/josm/gui/io/UploadPrimitivesTask.java

@@ -83 +83 @@
     }
 
-    protected MaxChangesetSizeExceededPolicy askMaxChangesetSizeExceedsPolicy() {
+    /**
+     * Prompt the user about how to proceed.
+     *
+     * @return the policy selected by the user
+     */
+    protected MaxChangesetSizeExceededPolicy promptUserForPolicy() {
         ButtonSpec[] specs = {
                 new ButtonSpec(
@@ -146 +151 @@
 
     /**
-     * Opens a new changeset.
-     */
-    protected void openNewChangeset() {
-        // make sure the current changeset is removed from the upload dialog.
-        ChangesetCache.getInstance().update(changeset);
-        Changeset newChangeSet = new Changeset();
-        newChangeSet.setKeys(this.changeset.getKeys());
-        this.changeset = newChangeSet;
-    }
-
-    protected boolean recoverFromChangesetFullException() throws OsmTransferException {
-        if (toUpload.getSize() - processedPrimitives.size() == 0) {
+     * Handles a server changeset full response.
+     * <p>
+     * Handles a server changeset full response by either aborting or opening a new changeset, if the
+     * user requested it so.
+     *
+     * @return true if the upload process should continue with the new changeset, false if the
+     *         upload should be interrupted
+     * @throws OsmTransferException "if something goes wrong."
+     */
+    protected boolean handleChangesetFullResponse() throws OsmTransferException {
+        if (processedPrimitives.size() == toUpload.getSize()) {
             strategy.setPolicy(MaxChangesetSizeExceededPolicy.ABORT);
             return false;
         }
         if (strategy.getPolicy() == null || strategy.getPolicy() == MaxChangesetSizeExceededPolicy.ABORT) {
-            strategy.setPolicy(askMaxChangesetSizeExceedsPolicy());
+            strategy.setPolicy(promptUserForPolicy());
         }
         switch(strategy.getPolicy()) {
         case AUTOMATICALLY_OPEN_NEW_CHANGESETS:
-            // prepare the state of the task for a next iteration in uploading.
-            closeChangesetIfRequired();
-            openNewChangeset();
+            Changeset newChangeSet = new Changeset();
+            newChangeSet.setKeys(changeset.getKeys());
+            closeChangeset();
+            this.changeset = newChangeSet;
             toUpload.removeProcessed(processedPrimitives);
             return true;
@@ -260 +265 @@
                     }
                     writer.uploadOsm(strategy, toUpload.getPrimitives(), changeset, getProgressMonitor().createSubTaskMonitor(1, false));
-
+                    // If the changeset was new, now it is open.
+                    ChangesetCache.getInstance().update(changeset);
                     // if we get here we've successfully uploaded the data. Exit the loop.
                     break;
@@ -277 +283 @@
                     case UPLOAD_DATA:
                         // Most likely the changeset is full. Try to recover and continue
-                        // with a new changeset, but let the user decide first (see
-                        // recoverFromChangesetFullException)
-                        if (recoverFromChangesetFullException()) {
+                        // with a new changeset, but let the user decide first.
+                        if (handleChangesetFullResponse()) {
                             continue;
                         }
                         lastException = e;
                         break uploadloop;
+                    case UPDATE_CHANGESET:
+                    case CLOSE_CHANGESET:
                     case UNSPECIFIED:
-                    case UPDATE_CHANGESET:
                     default:
                         // The changeset was closed when we tried to update it. Probably, our
@@ -292 +298 @@
                         // Rethrow exception - this will be handled later.
                         changeset.setOpen(false);
+                        ChangesetCache.getInstance().update(changeset);
                         throw e;
                     }
@@ -320 +327 @@
     }
 
+    /**
+     * Closes the changeset on the server and locally.
+     *
+     * @throws OsmTransferException "if something goes wrong."
+     */
+    private void closeChangeset() throws OsmTransferException {
+        if (changeset != null && !changeset.isNew() && changeset.isOpen()) {
+            try {
+                OsmApi.getOsmApi().closeChangeset(changeset, progressMonitor.createSubTaskMonitor(0, false));
+            } catch (ChangesetClosedException e) {
+                // Do not raise a stink, probably the changeset timed out.
+                Logging.trace(e);
+            } finally {
+                changeset.setOpen(false);
+                ChangesetCache.getInstance().update(changeset);
+            }
+        }
+    }
+
     private void closeChangesetIfRequired() throws OsmTransferException {
-        if (strategy.isCloseChangesetAfterUpload() && changeset != null && !changeset.isNew() && changeset.isOpen()) {
-            OsmApi.getOsmApi().closeChangeset(changeset, progressMonitor.createSubTaskMonitor(0, false));
-        }
-    }
-
-    @Override protected void finish() {
-
-        // depending on the success of the upload operation and on the policy for
-        // multi changeset uploads this will sent the user back to the appropriate
-        // place in JOSM, either
-        // - to an error dialog
-        // - to the Upload Dialog
-        // - to map editing
+        if (strategy.isCloseChangesetAfterUpload()) {
+            closeChangeset();
+        }
+    }
+
+    /**
+     * Depending on the success of the upload operation and on the policy for
+     * multi changeset uploads this will send the user back to the appropriate
+     * place in JOSM, either:
+     * <ul>
+     * <li>to an error dialog,
+     * <li>to the Upload Dialog, or
+     * <li>to map editing.
+     * </ul>
+     */
+    @Override
+    protected void finish() {
         GuiHelper.runInEDT(() -> {
             // if the changeset is still open after this upload we want it to be selected on the next upload

trunk/src/org/openstreetmap/josm/gui/io/UploadStrategySelectionPanel.java

@@ -11 +11 @@
 import java.awt.event.ActionEvent;
 import java.awt.event.ActionListener;
-import java.awt.event.FocusAdapter;
 import java.awt.event.FocusEvent;
+import java.awt.event.FocusListener;
 import java.awt.event.ItemEvent;
 import java.awt.event.ItemListener;
-import java.beans.PropertyChangeEvent;
-import java.beans.PropertyChangeListener;
 import java.util.EnumMap;
 import java.util.Map;
@@ -45 +43 @@
  * {@link #UPLOAD_STRATEGY_SPECIFICATION_PROP}.
  */
-public class UploadStrategySelectionPanel extends JPanel implements PropertyChangeListener {
+public class UploadStrategySelectionPanel extends JPanel {
 
     /**
@@ -57 +55 @@
     private final JosmTextField tfChunkSize = new JosmTextField(4);
     private final JPanel pnlMultiChangesetPolicyPanel = new JPanel(new GridBagLayout());
-    private final JRadioButton rbFillOneChangeset = new JRadioButton(
-            tr("Fill up one changeset and return to the Upload Dialog"));
-    private final JRadioButton rbUseMultipleChangesets = new JRadioButton(
-            tr("Open and use as many new changesets as necessary"));
+    private final JRadioButton rbFillOneChangeset = new JRadioButton();
+    private final JRadioButton rbUseMultipleChangesets = new JRadioButton();
     private JMultilineLabel lblMultiChangesetPoliciesHeader;
 
@@ -92 +88 @@
         gc.gridwidth = 1;
         gc.fill = GridBagConstraints.HORIZONTAL;
-        gc.insets = new Insets(0, 0, 3, 0);
+        gc.insets = new Insets(3, 3, 3, 3);
         gc.anchor = GridBagConstraints.FIRST_LINE_START;
         JRadioButton radioButton = rbStrategy.get(UploadStrategy.SINGLE_REQUEST_STRATEGY);
-        radioButton.setText(tr("Upload data in one request"));
-        pnl.add(radioButton, gc);
-        gc.gridx = 3;
-        gc.gridy = 1;
-        gc.weightx = 1.0;
-        gc.weighty = 0.0;
-        gc.gridwidth = 1;
-        pnl.add(lblNumRequests.get(UploadStrategy.SINGLE_REQUEST_STRATEGY), gc);
-
-        // -- chunked dataset strategy
-        gc.gridx = 0;
-        gc.gridy = 2;
-        gc.weightx = 0.0;
-        gc.weighty = 0.0;
-        radioButton = rbStrategy.get(UploadStrategy.CHUNKED_DATASET_STRATEGY);
-        radioButton.setText(tr("Upload data in chunks of objects. Chunk size: "));
+        radioButton.setText(tr("Upload all objects in one request"));
         pnl.add(radioButton, gc);
         gc.gridx = 2;
-        gc.gridy = 2;
+        gc.weightx = 1.0;
+        pnl.add(lblNumRequests.get(UploadStrategy.SINGLE_REQUEST_STRATEGY), gc);
+
+        // -- chunked dataset strategy
+        gc.gridy++;
+        gc.gridx = 0;
         gc.weightx = 0.0;
-        gc.weighty = 0.0;
-        gc.gridwidth = 1;
+        radioButton = rbStrategy.get(UploadStrategy.CHUNKED_DATASET_STRATEGY);
+        radioButton.setText(tr("Upload objects in chunks of size: "));
+        pnl.add(radioButton, gc);
+        gc.gridx = 1;
         pnl.add(tfChunkSize, gc);
-        gc.gridx = 3;
-        gc.gridy = 2;
-        gc.weightx = 0.0;
-        gc.weighty = 0.0;
-        gc.gridwidth = 1;
+        gc.gridx = 2;
         pnl.add(lblNumRequests.get(UploadStrategy.CHUNKED_DATASET_STRATEGY), gc);
 
         // -- single request strategy
+        gc.gridy++;
         gc.gridx = 0;
-        gc.gridy = 3;
-        gc.weightx = 0.0;
-        gc.weighty = 0.0;
         radioButton = rbStrategy.get(UploadStrategy.INDIVIDUAL_OBJECTS_STRATEGY);
         radioButton.setText(tr("Upload each object individually"));
         pnl.add(radioButton, gc);
-        gc.gridx = 3;
-        gc.gridy = 3;
-        gc.weightx = 0.0;
-        gc.weighty = 0.0;
-        gc.gridwidth = 1;
+        gc.gridx = 2;
         pnl.add(lblNumRequests.get(UploadStrategy.INDIVIDUAL_OBJECTS_STRATEGY), gc);
 
-        tfChunkSize.addFocusListener(new TextFieldFocusHandler());
         new ChunkSizeValidator(tfChunkSize);
 
@@ -159 +136 @@
         gc.fill = GridBagConstraints.HORIZONTAL;
         gc.anchor = GridBagConstraints.FIRST_LINE_START;
+        gc.insets = new Insets(3, 3, 3, 3);
         gc.weightx = 1.0;
         lblMultiChangesetPoliciesHeader = new JMultilineLabel(
-                tr("<html>There are <strong>multiple changesets</strong> necessary in order to upload {0} objects. " +
-                   "Which strategy do you want to use?</html>",
+                tr("<html><strong>Multiple changesets</strong> are necessary to upload {0} objects. " +
+                   "Please select a strategy:</html>",
                         numUploadedObjects));
         pnlMultiChangesetPolicyPanel.add(lblMultiChangesetPoliciesHeader, gc);
-        gc.gridy = 1;
+        gc.gridy++;
+        rbFillOneChangeset.setText(tr("Fill up one changeset and return to the Upload Dialog"));
         pnlMultiChangesetPolicyPanel.add(rbFillOneChangeset, gc);
-        gc.gridy = 2;
+        gc.gridy++;
+        rbUseMultipleChangesets.setText(tr("Open and use as many new changesets as necessary"));
         pnlMultiChangesetPolicyPanel.add(rbUseMultipleChangesets, gc);
 
@@ -185 +165 @@
         gc.weighty = 0.0;
         gc.anchor = GridBagConstraints.NORTHWEST;
-        gc.insets = new Insets(3, 3, 3, 3);
 
         add(buildUploadStrategyPanel(), gc);
         gc.gridy = 1;
         add(buildMultiChangesetPolicyPanel(), gc);
-
-        // consume remaining space
-        gc.gridy = 2;
-        gc.fill = GridBagConstraints.BOTH;
-        gc.weightx = 1.0;
-        gc.weighty = 1.0;
-        add(new JPanel(), gc);
 
         Capabilities capabilities = OsmApi.getOsmApi().getCapabilities();
@@ -313 +285 @@
             rbStrategy.get(UploadStrategy.SINGLE_REQUEST_STRATEGY).setEnabled(false);
             JRadioButton lbl = rbStrategy.get(UploadStrategy.SINGLE_REQUEST_STRATEGY);
-            lbl.setText(tr("Upload in one request not possible (too many objects to upload)"));
+            lbl.setEnabled(false);
             lbl.setToolTipText(tr("<html>Cannot upload {0} objects in one request because the<br>"
                     + "max. changeset size {1} on server ''{2}'' is exceeded.</html>",
@@ -334 +306 @@
             rbStrategy.get(UploadStrategy.SINGLE_REQUEST_STRATEGY).setEnabled(true);
             JRadioButton lbl = rbStrategy.get(UploadStrategy.SINGLE_REQUEST_STRATEGY);
-            lbl.setText(tr("Upload data in one request"));
+            lbl.setEnabled(true);
             lbl.setToolTipText(null);
             lblNumRequests.get(UploadStrategy.SINGLE_REQUEST_STRATEGY).setVisible(true);
@@ -369 +341 @@
     }
 
-    @Override
-    public void propertyChange(PropertyChangeEvent evt) {
-        if (evt.getPropertyName().equals(UploadedObjectsSummaryPanel.NUM_OBJECTS_TO_UPLOAD_PROP)) {
-            setNumUploadedObjects((Integer) evt.getNewValue());
-        }
-    }
-
-    static class TextFieldFocusHandler extends FocusAdapter {
-        @Override
-        public void focusGained(FocusEvent e) {
-            Component c = e.getComponent();
-            if (c instanceof JosmTextField) {
-                JosmTextField tf = (JosmTextField) c;
-                tf.selectAll();
-            }
-        }
-    }
-
     class ChunkSizeValidator extends AbstractTextComponentValidator {
         ChunkSizeValidator(JTextComponent tc) {
@@ -424 +378 @@
     }
 
-    class StrategyChangeListener extends FocusAdapter implements ItemListener, ActionListener {
+    class StrategyChangeListener implements FocusListener, ItemListener, ActionListener {
 
         protected void notifyStrategy() {
@@ -447 +401 @@
 
         @Override
+        public void focusGained(FocusEvent e) {
+            Component c = e.getComponent();
+            if (c instanceof JosmTextField) {
+                JosmTextField tf = (JosmTextField) c;
+                tf.selectAll();
+            }
+        }
+
+        @Override
         public void focusLost(FocusEvent e) {
             notifyStrategy();

trunk/src/org/openstreetmap/josm/gui/io/UploadedObjectsSummaryPanel.java

@@ -29 +29 @@
  */
 public class UploadedObjectsSummaryPanel extends JPanel {
-    /**
-     * The swing property name for the number of objects to upload
-     */
-    public static final String NUM_OBJECTS_TO_UPLOAD_PROP = UploadedObjectsSummaryPanel.class.getName() + ".numObjectsToUpload";
-
     /** the list with the added primitives */
     private PrimitiveList lstAdd;
@@ -146 +141 @@
             add(spDelete, gcList);
         }
-
-        firePropertyChange(NUM_OBJECTS_TO_UPLOAD_PROP, 0, getNumObjectsToUpload());
+        revalidate();
     }
 

trunk/src/org/openstreetmap/josm/gui/tagging/TagEditorModel.java

@@ -31 +31 @@
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetType;
 import org.openstreetmap.josm.tools.CheckParameterUtil;
+import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -54 +55 @@
     private transient OsmPrimitive primitive;
 
-    private EndEditListener endEditListener;
+    private transient EndEditListener endEditListener;
 
     /**
@@ -279 +280 @@
      */
     public void deleteTagNames(int... tagIndices) {
-        if (tags == null)
-            return;
         commitPendingEdit();
         for (int tagIdx : tagIndices) {
@@ -298 +297 @@
      */
     public void deleteTagValues(int... tagIndices) {
-        if (tags == null)
-            return;
         commitPendingEdit();
         for (int tagIdx : tagIndices) {
@@ -333 +330 @@
      */
     public void deleteTags(int... tagIndices) {
-        if (tags == null)
-            return;
         commitPendingEdit();
         List<TagModel> toDelete = Arrays.stream(tagIndices).mapToObj(tags::get).filter(Objects::nonNull).collect(Collectors.toList());
@@ -442 +437 @@
                 continue;
             }
+            boolean isKeyEmpty = Utils.isStripEmpty(tag.getName());
+            boolean isValueEmpty = Utils.isStripEmpty(tag.getValue());
+
+            // just the empty line at the bottom of the JTable
+            if (isKeyEmpty && isValueEmpty) {
+                continue;
+            }
 
             // tag name holds an empty key. Don't apply it to the selection.
-            if (!keepEmpty && (tag.getName().trim().isEmpty() || tag.getValue().trim().isEmpty())) {
+            if (!keepEmpty && (isKeyEmpty || isValueEmpty)) {
                 continue;
             }
-            result.put(tag.getName().trim(), tag.getValue().trim());
+            result.put(Utils.strip(tag.getName()), Utils.strip(tag.getValue()));
         }
         return result;
@@ -497 +499 @@
         // tag name holds an empty key. Don't apply it to the selection.
         //
-        if (tag.getName().trim().isEmpty())
+        if (Utils.isStripEmpty(tag.getName()))
             return null;
 
@@ -529 +531 @@
     public List<String> getKeys() {
         return tags.stream()
-                .filter(tag -> !tag.getName().trim().isEmpty())
+                .filter(tag -> !Utils.isStripEmpty(tag.getName()))
                 .map(TagModel::getName)
                 .collect(Collectors.toList());

trunk/src/org/openstreetmap/josm/gui/tagging/TagTable.java

@@ -36 +36 @@
 import org.openstreetmap.josm.tools.ImageProvider;
 import org.openstreetmap.josm.tools.Logging;
+import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -89 +90 @@
                 // we are at the end. Append an empty row and move the focus to its second column
                 String key = ((TagModel) model.getValueAt(row, 0)).getName();
-                if (!key.trim().isEmpty()) {
+                if (!Utils.isStripEmpty(key)) {
                     model.appendNewTag();
                     col = 0;
@@ -243 +244 @@
             }
             final int rowIdx = model.getRowCount()-1;
-            if (rowIdx < 0 || !((TagModel) model.getValueAt(rowIdx, 0)).getName().trim().isEmpty()) {
+            if (rowIdx < 0 || !Utils.isStripEmpty(((TagModel) model.getValueAt(rowIdx, 0)).getName())) {
                 model.appendNewTag();
             }

trunk/src/org/openstreetmap/josm/gui/widgets/JosmComboBoxModel.java

@@ -55 +55 @@
      *
      * @param element the element to get the index of
-     * @return an int representing the index position, where 0 is the first position
+     * @return the index of the first occurrence of the specified element in this model,
+     *         or -1 if this model does not contain the element
      */
     public int getIndexOf(E element) {
@@ -61 +62 @@
     }
 
+    protected void doAddElement(E element) {
+        if (element != null && (maxSize == -1 || getSize() < maxSize)) {
+            elements.add(element);
+        }
+    }
+
     //
     // interface java.lang.Iterable
@@ -79 +86 @@
     @Override
     public void addElement(E element) {
-        if (element != null && (maxSize == -1 || getSize() < maxSize)) {
-            elements.add(element);
-        }
+        doAddElement(element);
+        fireIntervalAdded(this, elements.size() - 1, elements.size() - 1);
     }
 
     @Override
     public void removeElement(Object elem) {
-        elements.remove(elem);
+        int index = elements.indexOf(elem);
+        if (elem == selected) {
+            if (index == 0) {
+                setSelectedItem(getSize() == 1 ? null : getElementAt(index + 1));
+            } else {
+                setSelectedItem(getElementAt(index - 1));
+            }
+        }
+        if (elements.remove(elem))
+            fireIntervalRemoved(this, index, index);
     }
 
@@ -115 +130 @@
         }
         elements.add(index, element);
+        fireIntervalAdded(this, index, index);
     }
 
@@ -167 +183 @@
      */
     public void addAllElements(Collection<E> elems) {
-        elems.forEach(e -> addElement(e));
+        int index0 = elements.size();
+        elems.forEach(e -> doAddElement(e));
+        int index1 = elements.size() - 1;
+        if (index0 <= index1)
+            fireIntervalAdded(this, index0, index1);
     }
 
@@ -178 +198 @@
      */
     public void addAllElements(Collection<String> strings, Function<String, E> buildE) {
-        strings.forEach(s -> addElement(buildE.apply(s)));
+        int index0 = elements.size();
+        strings.forEach(s -> doAddElement(buildE.apply(s)));
+        int index1 = elements.size() - 1;
+        if (index0 <= index1)
+            fireIntervalAdded(this, index0, index1);
     }
 
@@ -205 +229 @@
     public void removeAllElements() {
         if (!elements.isEmpty()) {
-            int firstIndex = 0;
             int lastIndex = elements.size() - 1;
             elements.clear();
             selected = null;
-            fireIntervalRemoved(this, firstIndex, lastIndex);
+            fireIntervalRemoved(this, 0, lastIndex);
         } else {
             selected = null;

trunk/src/org/openstreetmap/josm/io/ChangesetClosedException.java

@@ -31 +31 @@
     public static final String ERROR_HEADER_PATTERN = "The changeset (\\d+) was closed at (.*)";
 
+    /**
+     * Identifies when the changeset exception occurred.
+     */
     public enum Source {
         /**
@@ -43 +46 @@
          */
         UPLOAD_DATA,
+        /**
+         * The exception was thrown when we tried to close a changeset.  Probably the changeset
+         * already timed out on the server.
+         * @since 18283
+         */
+        CLOSE_CHANGESET,
         /**
          * Unspecified source
@@ -163 +172 @@
     }
 
+    /**
+     * Sets the source where the exception was thrown
+     *
+     * @param source the source where the exception was thrown
+     */
     public void setSource(Source source) {
         this.source = source == null ? Source.UNSPECIFIED : source;

trunk/src/org/openstreetmap/josm/io/OsmApi.java

@@ -388 +388 @@
                 }
             }
+        } catch (ChangesetClosedException e) {
+            e.setSource(ChangesetClosedException.Source.UPDATE_CHANGESET);
+            throw e;
         } catch (NumberFormatException e) {
             throw new OsmTransferException(errHandler.apply(ret), e);
@@ -529 +532 @@
             // send "\r\n" instead of empty string, so we don't send zero payload - workaround bugs in proxy software
             sendPutRequest("changeset/" + changeset.getId() + "/close", "\r\n", monitor);
+        } catch (ChangesetClosedException e) {
+            e.setSource(ChangesetClosedException.Source.CLOSE_CHANGESET);
+            throw e;
+        } finally {
             changeset.setOpen(false);
-        } finally {
             monitor.finishTask();
         }
@@ -565 +571 @@
             throws OsmTransferException {
         try {
+            ensureValidChangeset();
             monitor.beginTask("", list.size() * 2);
-            if (changeset == null)
-                throw new OsmTransferException(tr("No changeset present for diff upload."));
 
             initialize(monitor);
@@ -594 +599 @@
                     monitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)
             );
-        } catch (OsmTransferException e) {
+        } catch (ChangesetClosedException e) {
+            e.setSource(ChangesetClosedException.Source.UPLOAD_DATA);
             throw e;
         } catch (XmlParsingException e) {
@@ -752 +758 @@
                 case HttpURLConnection.HTTP_CONFLICT:
                     if (ChangesetClosedException.errorHeaderMatchesPattern(errorHeader))
-                        throw new ChangesetClosedException(errorBody, ChangesetClosedException.Source.UPLOAD_DATA);
+                        throw new ChangesetClosedException(errorBody, ChangesetClosedException.Source.UNSPECIFIED);
                     else
                         throw new OsmApiException(retCode, errorHeader, errorBody);

trunk/src/org/openstreetmap/josm/io/UploadStrategySpecification.java

@@ -151 +151 @@
 
     @Override
+    public String toString() {
+        return String.format("Strategy: %s, ChunkSize: %d, Policy: %s, Close after: %b",
+            strategy.toString(), chunkSize, policy == null ? "none" : policy.toString(), closeChangesetAfterUpload);
+    }
+
+    @Override
     public int hashCode() {
         return Objects.hash(strategy, chunkSize, policy, closeChangesetAfterUpload);