Modify

Opened 2 years ago

Closed 17 months ago

Last modified 17 months ago

#22534 closed defect (fixed)

"Download object" contents not populated most of the time

Reported by: richlv Owned by: team
Priority: normal Milestone: 23.06
Component: Core Version: tested
Keywords: Cc: sebastic

Description (last modified by skyper)

  1. Download a way
  2. Select this way, search for "child selected"
  3. Copy the found nodes
  4. File -> Download object

Rarely, the nodes are populated in the download field. Most often, nothing is populated.
This is a problem, as just pasting it in the "Object ID" field gets a format like "node 1431642962
node 1431646044
node ", newlines not displayed - and this format is not considered valid.

This _might_ be related to the number of the nodes in the way (and clipboard) - it might be failing for longer ways.

That is, exactly where this functionality would be the most useful.
Usecase: deleting a pretty long and windy way, so that lots and lots of data around it is not downloaded.

Cannot add status output, as it does not pass the "external link" check.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2022-10-31 17:29:20 +0100 (Mon, 31 Oct 2022)
Build-Date:2022-11-01 02:30:58
Revision:18583
Relative:URL: ^/trunk

Identification: JOSM/1.5 (18583 en_GB) Mac OS X 10.15.7
OS Build number: Mac OS X 10.15.7 (19H2026)
Memory Usage: 971 MB / 3641 MB (647 MB allocated, but free)
Java version: 1.8.0_351-b10, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.apple.laf.AquaLookAndFeel
Screen: Display 1127230987 1920×1080 (scaling 1.00×1.00) Display 69733382 1680×1050 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→16×16, 32×32→32×32
System property file.encoding: UTF-8
System property sun.jnu.encoding: UTF-8
Locale info: en_GB
Numbers with default locale: 1234567890 -> 1234567890
VM arguments: [-Djnlp.application.href=https://josm.openstreetmap.de/download/josm.jnlp, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlp.tk=awt, -D
jnlpx.jvm=
<java.home>/bin/java, -Djnlpx.splashport=-1, -Djnlpx.home=<java.home>/bin, -Djnlpx.remove=false, -Djnlpx.offline=false, -Djnlpx.relaunch=true, -Djnlpx.session.data=/var/folders/nl/flqxqsmj5q963r7tcnfrdt3c0000gn/T/session4062941147542902434, -Djnlpx.heapsize=NULL,NULL, -Djava.security.policy=file:<java.home>/lib/security/javaws.policy, -DtrustProxy=true, -Djnlpx.origFilenameArg=/Users/richlv/Library/Application Support/Oracle/Java/Deployment/cache/6.0/56/1ee8cfb8-72e8e992, -Dsun.awt.warmup=true, -Djava.security.manager]

Plugins:
+ HouseNumberTaggingTool (35951)
+ InfoMode (35978)
+ Mapillary (2.0.1)
+ PicLayer (1.0.2)
+ apache-commons (36003)
+ apache-http (35924)
+ buildings_tools (36011)
+ dataimport (35932)
+ ejml (35924)
+ geotools (36028)
+ imagery_offset_db (35978)
+ jackson (36006)
+ jaxb (35952)
+ jna (36005)
+ jts (36004)
+ measurement (35978)
+ opendata (36025)
+ pbf (35978)
+ photo_geotagging (35933)
+ reverter (36011)
+ utilsplugin2 (36011)

Map paint styles:
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Streets&zip=1

Attachments (1)

josm-status.txt (50.5 KB ) - added by richlv 2 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 by richlv, 2 years ago

Cannot add status output as a comment either, "Maximum number of external links per post exceeded".

comment:2 by taylor.smock, 2 years ago

You can change the advanced preference downloadprimitive.max-autopaste-length. The default is 2000, and the specified nodes are part of a way with 239 nodes, and the number of characters for that is 3823.

To fix this issue, someone will need to listen for a paste event of some kind, and then handle it.

For this specific use case, "deleting a pretty long and windy way" I would recommend using overpass instead of the Download object window. In order to safely delete the long way given the nodes, we have to get the referrers. This is 2 API calls per node, which on my system took ~.8s. This would take you ~3 minutes to download (191.2s).

The equivalent overpass query looks like this:

[out:xml][timeout:90];
(
  way(1038063792);
  >; // Recurse down (get the nodes)
  <; // Recurse up (get the node parents)
);
(._;>;);
out meta;

in reply to:  1 ; comment:3 by skyper, 2 years ago

Cc: sebastic added

Replying to richlv:

Cannot add status output as a comment either, "Maximum number of external links per post exceeded".

I do not understand why Debian changes the ReportBug action to point to reportbug(1). @sebastic might know more about the reason.
Anyway, please, have a look at the table at Show Status Report. We only need the first parts. If it still does not work, you could try to attach it as file. At least as compressed file it should work. Thanks.

in reply to:  3 comment:4 by sebastic, 2 years ago

Replying to skyper:

I do not understand why Debian changes the ReportBug action to point to reportbug(1).

Because the josm package in Debian is built differently from the upstream jars, issues specific to the package in Debian (not the upstream packaging) should be reported in the Debian BTS.

If you're happy to receive tickets for issues that may be caused by downstream changes, I'll happily drop the 'Report bug' changes to have it use Trac again.

comment:5 by taylor.smock, 2 years ago

Taking a look at https://salsa.debian.org/debian-gis-team/josm/-/tree/master/debian/patches, the only ones I have issues with are 00-build.patch (due to the commenting out of code -- I get the ivy stuff, but I don't know why you are commenting out other items) and 09-no-java-8.patch.

For 09-no-java-8.patch, it should be unnecessary due to trunk/build.xml@18587:80,220#L220 (we compile with --release 8). I didn't see any modification to the java.lang.version in 00-build.patch. Docs indicate that Java 17 should compile bytecode with the appropriate method calls for Java 8. But we are planning on dropping Java 8 support soon, so I'm not going to complain too much.

If you're happy to receive tickets for issues that may be caused by downstream changes, I'll happily drop the 'Report bug' changes to have it use Trac again.

Looking at bugs.debian.org, it looks like there have been a total of two bugs reported against the package.

My recommendation (note: other maintainers may disagree):

  • Add a line in the bug report window for the current maintainer of the JOSM Debian package. Use the username you have for JOSM trac (sebastic). That way a new contributor can look at the ticket and the code, and then reassign it to you if it didn't occur in our code.
Last edited 2 years ago by taylor.smock (previous) (diff)

in reply to:  5 ; comment:6 by sebastic, 2 years ago

Replying to taylor.smock:

Taking a look at https://salsa.debian.org/debian-gis-team/josm/-/tree/master/debian/patches, the only ones I have issues with are 00-build.patch (due to the commenting out of code -- I get the ivy stuff, but I don't know why you are commenting out other items) and 09-no-java-8.patch.

The Debian packages uses javac instead of ErrorProne, so we kept using the ant config for as before.

For 09-no-java-8.patch, it should be unnecessary due to trunk/build.xml@18587:80,220#L220 (we compile with --release 8). I didn't see any modification to the java.lang.version in 00-build.patch. Docs indicate that Java 17 should compile bytecode with the appropriate method calls for Java 8. But we are planning on dropping Java 8 support soon, so I'm not going to complain too much.

That may have changed in the mean time, when default-jdk in Debian switched from openjdk-8 to openjdk-11 running josm with openjdk-8-jre was broken.

The dependencies reflect this too:

Build-Depends: debhelper-compat (= 12),
               default-jdk (>= 2:1.9) | java9-sdk,
Depends: default-jre (>= 2:1.9) | java9-runtime,

If you're happy to receive tickets for issues that may be caused by downstream changes, I'll happily drop the 'Report bug' changes to have it use Trac again.

Looking at bugs.debian.org, it looks like there have been a total of two bugs reported against the package.

You're missing the archived bugreports:

https://bugs.debian.org/cgi-bin/pkgreport.cgi?archive=both;package=josm


My recommendation (note: other maintainers may disagree):

  • Add a line in the bug report window for the current maintainer of the JOSM Debian package. Use the username you have for JOSM trac (sebastic). That way a new contributor can look at the ticket and the code, and then reassign it to you if it didn't occur in our code.

That breaks when I get hit by a bus. The team is listed as Maintainer for the Debian package for that reason.

CC'ing me here in Trac works to get my attention, but not others in the Debian GIS team.

in reply to:  6 comment:7 by taylor.smock, 2 years ago

Replying to sebastic:

Replying to taylor.smock:

Looking at bugs.debian.org, it looks like there have been a total of two bugs reported against the package.

You're missing the archived bugreports:
https://bugs.debian.org/cgi-bin/pkgreport.cgi?archive=both;package=josm

I should have known. :(

My recommendation (note: other maintainers may disagree):

  • Add a line in the bug report window for the current maintainer of the JOSM Debian package. Use the username you have for JOSM trac (sebastic). That way a new contributor can look at the ticket and the code, and then reassign it to you if it didn't occur in our code.

That breaks when I get hit by a bus. The team is listed as Maintainer for the Debian package for that reason.
CC'ing me here in Trac works to get my attention, but not others in the Debian GIS team.

In that case, lets keep the status quo, but maybe there should be reportbug/reportbug-gtk integration instead of linking users to bugs.debian.org. I (personally) think [that just giving users a link] is useless -- many people won't bother to report a bug if they have to follow a few links to figure out how to report the bug.

Last edited 2 years ago by taylor.smock (previous) (diff)

by richlv, 2 years ago

Attachment: josm-status.txt added

comment:8 by richlv, 2 years ago

Status report: attached as a txt file.

Debian: not sure how that got pulled in, this was observed with a Mac version.

Overpass suggestion: that's excellent, thank you :)

Original issue: spent a long time trying and retrying this in various combinations... Can some warning be shown when autopaste fails because of "max-autopaste-length"?

comment:9 by skyper, 2 years ago

Description: modified (diff)

in reply to:  8 ; comment:10 by skyper, 2 years ago

Replying to richlv:

Status report: attached as a txt file.

Thanks, I've added the relevant parts. We do not need the last part (all preferences).

Debian: not sure how that got pulled in, this was observed with a Mac version.

Oh, sorry, I mixed it up with a different user. But with MacOS the Help > Report Bug action should work and I do not find too many external links in the relevant parts of the attached status report.

in reply to:  8 ; comment:11 by taylor.smock, 2 years ago

Replying to richlv:

Original issue: spent a long time trying and retrying this in various combinations... Can some warning be shown when autopaste fails because of "max-autopaste-length"?

Technically possible. We could (a) show the user a dialog asking them if they really want to continue, and note that overpass is probably a better solution or (b) just show a popup saying that the request could put unnecessary load on the OSM servers and note that overpass is probably a better solution.

I'm more inclined towards (b) especially if the user wants to get referrers, although we won't necessarily be able to tell if that is the current state of the server. Overpass does return a meta tag that looks like <meta osm_base="2022-11-30T13:20:51Z"/>, so we could probably work around it if the overpass data is significantly out of date. Maybe add an additional field server_time to bounds/primitives? But that has memory implications.

Anyway, we've had that paste behavior ever since r5786 (2013-03).

in reply to:  10 comment:12 by richlv, 2 years ago

Replying to skyper:

Replying to richlv:

Debian: not sure how that got pulled in, this was observed with a Mac version.

Oh, sorry, I mixed it up with a different user. But with MacOS the Help > Report Bug action should work and I do not find too many external links in the relevant parts of the attached status report.

I had already filled in the ticket, then recalled about the status report and copied it manually :)

in reply to:  11 comment:13 by richlv, 2 years ago

Replying to taylor.smock:

Technically possible. We could (a) show the user a dialog asking them if they really want to continue, and note that overpass is probably a better solution or (b) just show a popup saying that the request could put unnecessary load on the OSM servers and note that overpass is probably a better solution.

Either seems fine, as long as it's not mysteriously working sometimes, failing others.
If it's b), it could even show some copy-able examples to use in the Overpass download tab.

Currently Overpass data could be outdated (also intentionally, if user downloads older data to examine deleted entities), so that's probably a general concern when grabbing data from Overpass.

comment:14 by taylor.smock, 17 months ago

Resolution: fixed
Status: newclosed

In 18768/josm:

Fix #22534: Inform users when the clipboard has too much data

comment:15 by taylor.smock, 17 months ago

Milestone: 23.06

comment:16 by richlv, 17 months ago

Yay, thank you so much for this usability improvement :)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.