#18523 closed defect (fixed)
[PATCH] Upload dialog no longer displays number of changes in changeset after second upload
Reported by: | anonymous | Owned by: | Don-vip |
---|---|---|---|
Priority: | normal | Milestone: | 20.01 |
Component: | Core | Version: | tested |
Keywords: | template_report regression upload changeset | Cc: | taylor.smock, sanchi |
Description (last modified by )
What steps will reproduce the problem?
- Make some changes, upload changeset
- Make some more changes, upload changeset
What is the expected result?
Under the data source it should say
'Uploading # objects to # changeset using # request
Objects are uploaded to a new changeset. The changeset is going to be closed after this upload'
What happens instead?
The count and new changeset comment are missing from all following uploads after the first one. To see these again it is required to quit/restart josm, reload your data, redo the changes, and upload from there.
Reverting to a pervious version fixes this issue.
Please provide any additional information below. Attach a screenshot if possible.
URL:https://josm.openstreetmap.de/svn/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2020-01-02 22:34:59 +0100 (Thu, 02 Jan 2020) Build-Date:2020-01-02 21:52:31 Revision:15628 Relative:URL: ^/trunk Identification: JOSM/1.5 (15628 en) Mac OS X 10.14.6 OS Build number: Mac OS X 10.14.6 (18G1012) Memory Usage: 726 MB / 1820 MB (223 MB allocated, but free) Java version: 1.8.0_231-b11, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM Screen: Display 188945226 1920x1080, Display 69944052 2048x1152 Maximum Screen Size: 2048x1152 VM arguments: [-Djava.security.policy=file:<java.home>/lib/security/javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>/bin, -Djava.security.manager, -Djnlpx.origFilenameArg=${HOME}/Library/Application Support/Oracle/Java/Deployment/cache/6.0/56/1ee8cfb8-4e1b3343, -Djnlpx.remove=false, -Dsun.awt.warmup=true, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlpx.heapsize=NULL,2048m, -Dmacosx.jnlpx.dock.name=JOSM, -Dmacosx.jnlpx.dock.icon=${HOME}/Library/Application Support/Oracle/Java/Deployment/cache/6.0/25/4c122699-23c26c79.icns, -Djnlp.application.href=https://josm.openstreetmap.de/download/josm.jnlp , -Djnlpx.jvm="<java.home>/bin/java"] Plugins: + auto_tools (73) + markseen (14) + osm-obj-info (56) + scripting (30796) + utilsplugin2 (35248) Last errors/warnings: - W: java.nio.file.NoSuchFileException: ${HOME}/Downloads/kaart.durazno.validator.mapcss - E: Skipping to the next rule, because of an error: - E: org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.ParseException: Encountered " "(" "( "" at line 9, column 14. - W: No configuration settings found. Using hardcoded default values for all pools. - W: Region [TMS_BLOCK_v2] Resetting cache - E: Communication with OSM server failed - org.openstreetmap.josm.gui.widgets.HtmlPanel[,0,0,0x0,invalid,layout=java.awt.BorderLayout,alignmentX=0.0,alignmentY=0.0,border=,flags=9,maximumSize=,minimumSize=,preferredSize=] - W: Proposed rect has such extreme aspect ratio that it would be zero-width at preferredZoom
Attachments (13)
Change History (59)
by , 5 years ago
Attachment: | changeset_issue.png added |
---|
comment:1 by , 5 years ago
Keywords: | regression upload changeset added |
---|---|
Version: | → tested |
comment:2 by , 5 years ago
Cc: | added |
---|---|
Description: | modified (diff) |
Summary: | Dialogue box no longer displays number of changes in changeset after second upload → Upload dialog no longer displays number of changes in changeset after second upload |
comment:3 by , 5 years ago
Description: | modified (diff) |
---|
comment:4 by , 5 years ago
comment:5 by , 5 years ago
Summary: | Upload dialog no longer displays number of changes in changeset after second upload → [PATCH] Upload dialog no longer displays number of changes in changeset after second upload |
---|
comment:6 by , 5 years ago
Milestone: | → 20.01 |
---|
comment:7 by , 5 years ago
Ping?
I'd like to know if I should try to find a better solution, or if switching to GridBagLayout
is acceptable.
comment:8 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
I guess GridBagLayout is fine. Let's give it a try.
follow-ups: 11 43 comment:10 by , 5 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
by , 5 years ago
Attachment: | layoutbug.jpg added |
---|
comment:11 by , 5 years ago
Replying to Don-vip:
Nope. I didn't have any problem before applying this patch and now the layout does not work anymore for me:
The UI was working for me (as in, I didn't have it cut off).
If I comment out just about any of the adds
in build
, buildUploadCommentPanel
, pnlUploadParameterSummary
, or cbRequestReview
, it works for me again without any of the patches here applied.
Can you check if changing the GBC.HORIZONTAL
to GBC.BOTH
works? (in build
). And vice versa? (also, I've noticed that I've had to run ant clean dist
to get some changes to show up, but that probably wasn't your problem).
by , 5 years ago
Attachment: | 2020-01-24-071411_598x465_scrot.png added |
---|
by , 5 years ago
Attachment: | Screen Shot 2020-01-24 at 7.25.45 AM.png added |
---|
by , 5 years ago
Attachment: | all.do_not_apply.patch added |
---|
All of the modifications in my josm/core directory
comment:12 by , 5 years ago
The screen shot is from the 18523.GBC_BOTH.patch
.
I've uploaded a svn diff
of everything in my josm/core
directory as well.
I think the only things that would affect this are between lines 2805 and 2832, and the diff in src/org/openstreetmap/josm/gui/io/UploadDialog.java
was just to ensure that I always got a new upload dialog for testing purposes, but I'm not encountering the issue at all, which means I cannot verify if I have fixed it or not.
I've also tested in a josm
directory with literally nothing else applied (just attachment:18523.GBC_BOTH.patch), and that didn't show the problem.
comment:13 by , 5 years ago
@simon04:
I reproduced your screenshot on Fedora 31.
The 18523.GBC_BOTH.patch fixed it for me. Can you check if it fixes it for you as well?
Funny things are still happening though.
@Don-vip, should I make a patch that reverts the UI, but leaves an advanced preference for getting sources automatically, until I (or someone else) figures out the UI issue?
comment:14 by , 5 years ago
Priority: | normal → critical |
---|
(This is a pretty bad bug. I wonder why we don't receive more bug reports.)
comment:16 by , 5 years ago
by , 5 years ago
comment:19 by , 5 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Still have the same problem as Klumbumbus in comment 10. Switching to another tab and back solves the issue but on all uploads except the first one the source text box is too small in the beginning.
Relative:URL: ^/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2020-01-28 00:39:43 +0100 (Tue, 28 Jan 2020) Revision:15791 Build-Date:2020-01-28 02:30:55 URL:https://josm.openstreetmap.de/svn/trunk
by , 5 years ago
Attachment: | 18523.manually_set_minimum_size.patch added |
---|
comment:21 by , 5 years ago
Can everyone having the problem shown in comment:10 try the attachment:18523.manually_set_minimum_size.patch ? From what I understand of Swing, it shouldn't be necessary, but its working for me on multiple trial uploads.
I didn't notice it with the previous patch because I had changed UploadDialog
to always return a new upload dialog so I didn't have to restart JOSM every time I made a change.
follow-ups: 23 24 30 comment:22 by , 5 years ago
Yes, fixes the problem for me. My way to reproduce it:
1) Create node with amenity=parking
2) "Upload data" (dialog is OK)
3) press Esc
4) "upload data" again (dialog is not OK)
comment:23 by , 5 years ago
That is basically how I reproduced it once I was no longer getting a "new" upload dialog every time.
comment:25 by , 5 years ago
Cc: | added |
---|
follow-up: 28 comment:26 by , 5 years ago
With the follwoing parts of r15752 reverted, I cannot reproduce according to comment:22
-
src/org/openstreetmap/josm/gui/io/BasicUploadSettingsPanel.java
diff --git a/src/org/openstreetmap/josm/gui/io/BasicUploadSettingsPanel.java b/src/org/openstreetmap/josm/gui/io/BasicUploadSettingsPanel.java index 2b6a61ec7..1135540d1 100644
a b 3 3 4 4 import static org.openstreetmap.josm.tools.I18n.tr; 5 5 6 import java.awt.BorderLayout; 6 7 import java.awt.GridBagLayout; 7 8 import java.awt.event.ActionEvent; 8 9 import java.awt.event.ActionListener; … … protected void discardAllUndoableEdits() { 176 177 } 177 178 178 179 protected void build() { 179 setLayout(new GridBagLayout());180 setLayout(new BorderLayout()); 180 181 setBorder(BorderFactory.createEmptyBorder(3, 3, 3, 3)); 181 add(buildUploadCommentPanel(), GBC.eol().fill(GBC.BOTH));182 add(pnlUploadParameterSummary, GBC.eol().fill(GBC.BOTH));183 add(cbRequestReview, GBC.eol().fill(GBC.BOTH));182 add(buildUploadCommentPanel(), BorderLayout.NORTH); 183 add(pnlUploadParameterSummary, BorderLayout.CENTER); 184 add(cbRequestReview, BorderLayout.SOUTH); 184 185 cbRequestReview.addItemListener(e -> changesetReviewModel.setReviewRequested(e.getStateChange() == ItemEvent.SELECTED)); 185 186 } 186 187
comment:27 by , 5 years ago
comment:28 by , 5 years ago
Switching back to BorderLayout
also works on Mac (at least on this one), without the 18523.manually_set_minimum_size.patch.
The hard part about this bug (from my perspective) is that different machines are acting differently with respect to the UI.
For example, I wasn't seeing the issue that Don-vip was seeing in comment:10 on Mac, while I saw it on Fedora.
I'll post a patch that reverts the UI portion, while keeping an advanced option around (for whenever I or someone else figures out the UI issues).
by , 5 years ago
Attachment: | 18523.ui_revert.patch added |
---|
Revert UI while leaving logic (advanced pref upload.source.obtainautomatically
can be set)
comment:29 by , 5 years ago
That should fix the UI issue while keeping the functionality for an advanced preference.
Possible issues:
- I left the code for generating the UI in JOSM, so there are some unused variables (I want to see if I can get something that satisfies everyone)
- People may be confused about missing the checkbox while still getting source automatically
My initial patch was literally setting the minimum size for the affected panel to 80
, which the manually_set_minimum_size patch did, but the latter was a bit better in terms of (hopefully) dealing with HiDPI.
follow-up: 31 comment:30 by , 5 years ago
Replying to GerdP:
Yes, fixes the problem for me. My way to reproduce it:
1) Create node with amenity=parking
2) "Upload data" (dialog is OK)
3) press Esc
4) "upload data" again (dialog is not OK)
I could reproduce this with r15793. But I found an interesting detail (on Windows):
When I go to the border of the dialog where the resize mouse pointer appears and I just klick, the layout will be corrected!
comment:31 by , 5 years ago
I could reproduce this with r15793. But I found an interesting detail (on Windows):
When I go to the border of the dialog where the resize mouse pointer appears and I just klick, the layout will be corrected!
In mac it happens the same
comment:32 by , 5 years ago
The following also fixed the problem for me. Yes, all I am doing is calling super.getPreferredSize()
-- I was going to check and see if that was giving odd results as well. It turns out that is relatively stable. (On initial show getPreferredSize
gets called four times, with 190x434
, 222x434
, and 299x434
being the different minimum sizes, after initial show it gets called once, which returns 222x434
-- heightx
width).
-
src/org/openstreetmap/josm/gui/io/UploadDialog.java
432 432 static final class CompactTabbedPane extends JTabbedPane { 433 433 @Override 434 434 public Dimension getPreferredSize() { 435 super.getPreferredSize(); 435 436 // make sure the tabbed pane never grabs more space than necessary 436 437 return super.getMinimumSize(); 437 438 }
I hate working on UI stuff. I don't get why there would be a different behavior if you call super.getPreferredSize()
before super.getMinimumSize()
versus just super.getMinimumSize()
-- it makes no logical sense to me. There is probably something somewhere in the code that will clear it up though.
follow-up: 34 comment:33 by , 5 years ago
Maybe the tabs with possibly different height is the problem. Switching to "Changesets" or "Advanced" adjusts the height of the lower part of the dialog, but only after changing a setting there.
comment:34 by , 5 years ago
Replying to skyper:
Maybe the tabs with possibly different height is the problem. Switching to "Changesets" or "Advanced" adjusts the height of the lower part of the dialog, but only after changing a setting there.
Possibly -- the tabs changing height appears to be a byproduct of GridBagLayout -- BorderLayout doesn't have that problem (see r15752). I've switched that back (essentially did the patch from comment:28), and it doesn't occur anymore.
I'm also not seeing the UI issue from comment:10 when I run JOSM from Eclipse, but I do see it when I run from a jar file.
That being said, my observance from comment:32 also fixes the UI issue (when run from a jar file).
I don't know why.
super.getPreferredSize(); // This probably fixes #18523. Don't know why. Don't know how. It just does.
by , 5 years ago
Attachment: | 18523.revert_gridbaglayout_add_seemingly_unnecessary_call.patch added |
---|
Go from GridBagLayout -> BorderLayout and add a totally unnecessary method call (super.getPreferredSize()
), if it didn't seem to fix a UI issue.
follow-ups: 36 38 42 comment:35 by , 5 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
In 15802/josm:
comment:36 by , 5 years ago
Replying to Don-vip:
In 15802/josm:
Did it really fix it for everyone? I probably should have indicated that people having the issue should test it.
follow-up: 39 comment:38 by , 5 years ago
Replying to Don-vip:
In 15802/josm:
Work like a charm now thought the height always stays the same.
Relative:URL: ^/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2020-01-31 00:23:28 +0100 (Fri, 31 Jan 2020) Revision:15803 Build-Date:2020-01-31 02:30:58 URL:https://josm.openstreetmap.de/svn/trunk Identification: JOSM/1.5 (15803 en) Linux Debian GNU/Linux 10 (buster)
comment:39 by , 5 years ago
Replying to skyper:
Work like a charm now thought the height always stays the same.
r15598 had the same behavior (at least on my machine). So we are back to the previous behavior.
I still don't know why the call to getPreferredSize
when I did nothing with the output fixed the issue. Java bug maybe? I would have expected the behavior of getMinimumSize
to be the same, regardless of whether or not getPreferredSize
was called first.
comment:40 by , 5 years ago
I also cannot reproduce the problem with r15803 (tried Oracle Java and OpenJDK 1.8)
comment:41 by , 5 years ago
Works fine for me
URL:https://josm.openstreetmap.de/svn/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2020-01-31 00:23:28 +0100 (Fri, 31 Jan 2020) Build-Date:2020-01-31 02:30:58 Revision:15803 Relative:URL: ^/trunk Identification: JOSM/1.5 (15803 de) Windows 10 64-Bit OS Build number: Windows 10 Pro 1903 (18362) Memory Usage: 952 MB / 1820 MB (531 MB allocated, but free) Java version: 1.8.0_241-b07, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM Screen: \Display0 1680x1050 Maximum Screen Size: 1680x1050 VM arguments: [-Djava.security.manager, -Djava.security.policy=file:<java.home>\lib\security\javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>\bin, -Djnlpx.origFilenameArg=C:\Program Files (x86)\josm-latest.jnlp, -Djnlpx.remove=false, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlpx.heapsize=NULL,2048m, -Djnlpx.splashport=53153, -Djnlpx.jvm=<java.home>\bin\javaw.exe] Dataset consistency test: No problems found
comment:42 by , 5 years ago
follow-up: 44 comment:43 by , 4 years ago
Replying to Don-vip:
Nope. I didn't have any problem before applying this patch and now the layout does not work anymore for me:
Same issue for me on latest dev version (r17023). Try creating a changeset without closing it (uncheck it in 'Changesets' tab). Next time you upload something it will look like Don-vip's screenshot. Should I reopen the issue?
follow-up: 45 comment:44 by , 4 years ago
Replying to gaben:
Same issue for me on latest dev version (r17023). Try creating a changeset without closing it (uncheck it in 'Changesets' tab). Next time you upload something it will look like Don-vip's screenshot. Should I reopen the issue?
Is it #19319?
As this ticket was closed months ago, it is probably better to open a new ticket as follow-up and link to and from this one.
It looks like the minimum height for the
pnlUploadParameterSummary
is being set to0
. I've got a quick and dirty workaround, but I'm not certain what the best way workaround would actually be.