[Patch] Trimming too agressive on backspace (regression)

[Famlam]

What steps will reproduce the problem?

1. As a changeset comment, type "test tes test" (keep the cursor position behind the final "t")
2. Notice you made a typo and press backspace 5 times
3. Notice JOSM makes the space disappear before you press backspace on it, causing you to delete one letter too much

What is the expected result?

As comment: "test tes"

What happens instead?

As comment: "test te"

Please provide any additional information below. Attach a screenshot if possible.

Seems a regression of the end-of-May release; pretty sure wasn't present in any previous version

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-06-02 22:03:39 +0200 (Wed, 02 Jun 2021)
Build-Date:2021-06-02 20:11:30
Revision:17919
Relative:URL: ^/trunk

Identification: JOSM/1.5 (17919 nl) Windows 10 64-Bit
OS Build number: Windows 10 Home 2009 (19043)
Memory Usage: 589 MB / 1820 MB (427 MB allocated, but free)
Java version: 1.8.0_291-b10, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920×1080 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→32×32, 32×32→32×32
System property file.encoding: Cp1252
System property sun.jnu.encoding: Cp1252
Locale info: nl_NL
Numbers with default locale: 1234567890 -> 1234567890
Dataset consistency test: No problems found

Plugins:
+ OpeningHoursEditor (35640)
+ SimplifyArea (35640)
+ imagery_offset_db (35640)
+ pt_assistant (1ff2e15)
+ reverter (35732)
+ tageditor (35640)
+ turnlanes-tagging (288)
+ undelete (35640)
+ utilsplugin2 (35691)

Map paint styles:
- https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/PublicTransport&zip=1
+ %UserProfile%\Documents\tijdelijke bestanden\josm-eigen.mappaint.mapcss
+ https://josm.openstreetmap.de/josmfile?page=Styles/Sidewalks&zip=1

Validator rules:
+ %UserProfile%\Documents\tijdelijke bestanden\josm-eigen.validator.mapcss

Last errors/warnings:
- 00015.987 E: Lokaliseren van afbeelding 'bus.png' mislukt

[gaben]

Seems a regression of the end-of-May release; pretty sure wasn't present in any previous version

You can download previous versions and narrow down to the revision number in which occurred first.

[Famlam]

17833: OK (end of april release)
17892: OK
17894: (broken: IllegalStateException - can't test)
17895: (broken: IllegalStateException - can't test)
17903: FAILS (i.e., has this issue)

So something between commits [17893:17903] caused this regression.
I would suspect r17894 as it seems to be the only one changing something to the comment text area?

[skyper]

Additionally, the cursor jumps to the end of line if deleting letters at the front one by one and reaching a white space.

[Klumbumbus]

This input box has gotten a very bad behavior. A fix would be highly appreciated.

[Bjoeni]

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

@@ -410 +410 @@
         public void stateChanged(ChangeEvent e) {
             if (!(e.getSource() instanceof ChangesetCommentModel)) return;
             String newComment = ((ChangesetCommentModel) e.getSource()).getComment();
-            if (!destination.getText().equals(newComment)) {
+            if (!destination.getText().trim().equals(newComment)) {
                 destination.setText(newComment);
             }
         }

src/org/openstreetmap/josm/gui/io/ChangesetCommentModel.java

@@ -24 +24 @@
      */
     public void setComment(String comment) {
         String oldValue = this.comment;
-        this.comment = comment == null ? "" : comment;
+        this.comment = comment == null ? "" : comment.trim();
         if (!Objects.equals(oldValue, this.comment)) {
             fireStateChanged();
         }

That whole HistoryComboBox (Edit: actually it's not the HistoryComboBox but just the way it's handled in BasicUploadSettingsPanel) is pretty messy (as per usual when trying to keep two data sources in sync). Typing something in the textbox causes the combobox to be updated and send an event back to the textbox to be updated, which then updates the combobox again (and then it stops, because nothing changes).
I actually don't even understand why the combobox needs to be updated on the fly at all: In my opinion the combobox should just be filled once from the preferences, and it should be sufficient to only keep the current comment in the textbox. That might also fix #20690 and should be looked at there.

But for now, the patch above should fix the issue in this ticket.

Edit2: After having another look at it, I get what the intention was (reusing the existing TagEditorPanel / TagModel). I still don't think it's a good idea to keep different data sources in sync manually.

[Don-vip]

In 17995/josm:

fix #20969 - fix trimming/synchronization issue in changeset upload comment input field (patch by Bjoeni)