search

hotosm / osm-tasking-manager2

Designed and built for Humanitarian OpenStreetMap Team collaborative emergency/disaster mapping, the OSM Tasking Manager 2.0 divides an area into individual squares that can be rapidly mapped by thousands of volunteers.
http://tasks.hotosm.org
Other
425 stars 156 forks source link

Difficult to read changeset comments to JOSM #513

Closed bgirardot closed 9 years ago

bgirardot commented 9 years ago

I haven't figured out exactly where the issue is yet, but I see that now in JOSM the suggested changeset comments are difficult to read due to url encoding:

DRC%2C%20%23hotosm-ebola-drc-661%2C%20%23MapGive%2C%20source%3DWorldView-2%2C%20DigitalGlobe%2C%20NextView%2C%208%20March%202011%20or%2016%20September%202014

In case github decodes the example above you can see them here: https://www.openstreetmap.org/changeset/27800033

This might be one we have to put a ticket into JOSM for to ask them to decode the URL encoding.

Greenie10 commented 9 years ago

I also noticed this was happening this morning. So I cut and pasted the text from the tasking manager into sublime and cut and pasted again into josm, and it was ok then - but obviously can’t expect people to do this :)

On 30 Dec 2014, at 13:47, bgirardot notifications@github.com wrote:

I haven't figured out exactly where the issue is yet, but I see that now in JOSM the suggested changeset comments are difficult to read due to url encoding:

DRC%2C%20%23hotosm-ebola-drc-661%2C%20%23MapGive%2C%20source%3DWorldView-2%2C%20DigitalGlobe%2C%20NextView%2C%208%20March%202011%20or%2016%20September%202014

In case github decodes the example above you can see them here: https://www.openstreetmap.org/changeset/27800033 https://www.openstreetmap.org/changeset/27800033 This might be one we have to put a ticket into JOSM for to ask them to decode the URL encoding.

— Reply to this email directly or view it on GitHub https://github.com/hotosm/osm-tasking-manager2/issues/513.

bgirardot commented 9 years ago

It happens in both JOSM 7777 and 7906. I think maybe it needs to be a josm ticket, but I have to look a little more, I have never looked at the code that passes information to JOSM.

ethan-nelson commented 9 years ago

You might try using bleach.clean(JOSMCOMMENT, strip=True) to wipe out the html tags when they're sent. JOSM most likely can't process html formatting and therefore prints this.

pgiraud commented 9 years ago

This is not due to HTML tags actually but to URL encoding instead. See https://github.com/hotosm/osm-tasking-manager2/issues/449 I'm afraid this patch hasn't been tested in a real case. It looks like JOSM will have to do some URL encoding.

pyrog commented 9 years ago

Same issue with http://tasks.hotosm.org/project/837

http://127.0.0.1:8111/load_and_zoom?left=125.44739&bottom=11.99634&right=125.45288&top=12.00171&changeset_comment=%2523hotosm-project-837%2520-%2520Typhoon%2520Ruby%2520-%2520%2523SkyEye%2520%2523RubyPH%2520&changeset_source=tms%5B23%5D%3Ahttp%3A%2F%2Fimagery.openstreetmap.fr%2Ftms%2F1.0.0%2Frubyph%2F%7Bzoom%7D%2F%7Bx%7D%2F%7By%7D.png

I opened the ticket #10975

It looks like JOSM will have to do some URL encoding.

URL decoding :wink:

pierzen commented 9 years ago

source is added to the comment while there is a source field at least in JOSM. This make duplication.

pyrog commented 9 years ago

I found a trick: do not URL encoding the comment parameter. [http://127.0.0.1:8111/load_and_zoom?…](http://127.0.0.1:8111/load_and_zoom?left=125.44739&bottom=11.99634&right=125.45288&top=12.00171&changeset_comment=hotosm-project-837 - Typhoon Ruby - SkyEye RubyPH&changeset_source=tms%5B23%5D%3Ahttp%3A%2F%2Fimagery.openstreetmap.fr%2Ftms%2F1.0.0%2Frubyph%2F%7Bzoom%7D%2F%7Bx%7D%2F%7By%7D.png)

http://127.0.0.1:8111/load_and_zoom?left=125.44739&bottom=11.99634&right=125.45288&top=12.00171&changeset_comment=hotosm-project-837 - Typhoon Ruby - SkyEye RubyPH&changeset_source=tms%5B23%5D%3Ahttp%3A%2F%2Fimagery.openstreetmap.fr%2Ftms%2F1.0.0%2Frubyph%2F%7Bzoom%7D%2F%7Bx%7D%2F%7By%7D.png

But It doesn't work (at least) with the # character

pyrog commented 9 years ago

In fact, there is two bugs:

Changeset comment encoded only once (without #):

Changeset comment encoded only once (containing #):

pyrog commented 9 years ago

"bug" in JOSM in /src/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandler.java#L199

    /**
     * Parse the request parameters as key=value pairs.
     * The result will be stored in {@code this.args}.
     *
     * Can be overridden by subclass.
     */
    protected void parseArgs() {
        try {
            String req = URLDecoder.decode(this.request, "UTF-8");
            HashMap<String, String> args = new HashMap<>();
            if (req.indexOf('?') != -1) {
                String query = req.substring(req.indexOf('?') + 1);
                if (query.indexOf('#') != -1) {
                            query = query.substring(0, query.indexOf('#'));
                        }
                String[] params = query.split("&", -1);
                for (String param : params) {
                    int eq = param.indexOf('=');
                    if (eq != -1) {
                        args.put(param.substring(0, eq), param.substring(eq + 1));
                    }
                }
            }
            this.args = args;
        } catch (UnsupportedEncodingException ex) {
            throw new IllegalStateException(ex);
        }
    }
pyrog commented 9 years ago

Bug in OSM TM probably in /osmtm/templates/task.mako#L89

var changeset_comment = "${quote(project.changeset_comment.encode('utf8'), '')}";

But be careful to check if there is no new issue with other editors in /osmtm/static/js/project.js#L375

    function getLink(options) {
      var bounds = options.bounds,
          so = new L.LatLng(bounds[0], bounds[1]),
          ne = new L.LatLng(bounds[2], bounds[3]),
          zoom = lmap.getBoundsZoom(new L.LatLngBounds(so, ne));
          c = options.centroid;
      switch (options.protocol) {
        case 'lbrt':
        if (typeof imagery_url != "undefined" && imagery_url !== '') {
          source=imagery_url;
        } else {
          source="Bing";
        }
        return options.base + $.param({
          left: roundd(bounds[0],5),
          bottom: roundd(bounds[1],5),
          right: roundd(bounds[2],5),
          top: roundd(bounds[3],5),
          changeset_comment: changeset_comment,
          changeset_source: source
        });
        case 'llz':
        return options.base + $.param({
          lon: roundd(c[0],5),
          lat: roundd(c[1],5),
          zoom: zoom
        });
        case 'id':
        return options.base + '#map=' +
        [zoom, c[1], c[0]].join('/') +
        '&comment=' + changeset_comment;
      }
    }
pgiraud commented 9 years ago

There's seem to have been reported back to JOSM, but here's their answer. https://josm.openstreetmap.de/ticket/10975 We should definitely have a better look at this.

ethan-nelson commented 9 years ago

I can run some tests tomorrow

ethan-nelson commented 9 years ago

It looks like the issue stems from jQuery's $.param call. Pull request coming soon.

pgiraud commented 9 years ago

@ethan-nelson I merged your pull request. Can you confirm that we can now close this issue? Thanks for your great contribution by the way.

ethan-nelson commented 9 years ago

@pgiraud the core of this ticket (changeset comments) is solved by the pull request. This is not a JOSM issue. So, it should be closable. :sunglasses: