Modify

Opened 3 months ago

Last modified 8 weeks ago

#15645 reopened enhancement

Should offer a way to fix objects with invalid characters

Reported by: naoliv Owned by: team
Priority: minor Milestone:
Component: Core validator Version:
Keywords: Cc:

Description

Validate the attached example and see a warning about Tag value contains character with code less than 0x20 - Key 'description' invalid.

But it's not clear what exactly is the problem nor what the user needs to do to fix this.

With Ctrl+i I can see that maybe there is a newline (or similar character) after permitido:

    "description"="Acesso apenas por barco particular, não é permitido
concentrações elevadas de visitantes."

which is confirmed if we take a look at the XML:
<tag k='description' v='Acesso apenas por barco particular, não é permitido&#xA;concentrações elevadas de visitantes.' />

But if we simply edit the key we can't see anything wrong:

https://i.imgur.com/KemUTP3.png

Two possibilities here:

  • JOSM could display the invalid character if we edit the value
  • JOSM could offer to fix it (by just removing it)

JOSM:

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2017-12-05 00:58:56 +0100 (Tue, 05 Dec 2017)
Revision:13194
Build-Date:2017-12-05 02:33:31
URL:http://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (13194 en) Linux Debian GNU/Linux testing (buster)
Memory Usage: 810 MB / 7168 MB (659 MB allocated, but free)
Java version: 9.0.1+11-Debian-1, Oracle Corporation, OpenJDK 64-Bit Server VM
Screen: :0.0 1600x900, :0.1 1280x1024
Maximum Screen Size: 1600x1024
Java package: openjdk-9-jre:amd64-9.0.1+11-1
Java ATK Wrapper package: libatk-wrapper-java:all-0.33.3-13
VM arguments: [--add-modules=java.activation,java.se.ee, -Dawt.useSystemAAFontSettings=on]
Dataset consistency test: No problems found

Attachments (1)

example.osm (3.3 KB) - added by naoliv 3 months ago.

Download all attachments as: .zip

Change History (16)

Changed 3 months ago by naoliv

Attachment: example.osm added

comment:1 Changed 3 months ago by Don-vip

Type: defectenhancement

comment:2 Changed 2 months ago by Don-vip

Milestone: 17.12

comment:3 Changed 2 months ago by Don-vip

Resolution: fixed
Status: newclosed

In 13226/josm:

fix #15645 - autofix objects with invalid characters

comment:4 Changed 2 months ago by aceman

Is just removing the characters the best way to do it?
In the given example, replacing by space ' ' could be more useful. Is the user given the chance to review the result?

comment:5 Changed 2 months ago by cmuelle8

Shouldn't this be server side checked?

If the server allows that, I'd say josm should not modify the string.

It is easy to generate incompatibilities with other editors else.

What about displaying the string with escaped characters if and only if it contains unprintable parts?
That way mappers are made aware of it and can decide themselves whether to keep or replace them.

If this is a general issue, it should probably have a rule in the validator.

comment:6 in reply to:  5 Changed 2 months ago by cmuelle8

Replying to cmuelle8:

What about displaying the string with escaped characters if and only if it contains unprintable parts?

(.. with an extra label below or above the editable field to be more specific)

comment:7 Changed 2 months ago by Klumbumbus

Resolution: fixed
Status: closedreopened

comment:8 Changed 2 months ago by Don-vip

Resolution: fixed
Status: reopenedclosed

Deleting characters is the simplest and easy way to fix the problem. You can check the string and if the fix wasn't the good one you can edit the string manually. I won't add any further complex mechanism/UI for this.

comment:9 Changed 2 months ago by cmuelle8

Since when is josm after the simplest and easiest way to fix problems?
I'd join in going for asap as in "as simple as possible".

The problem here is that we do not know if the issue indeed is a problem.

We do not have a clear spec on how to handle the values,
i.e. who says (or where does it say) that the key value strictly takes one line of text only?

As long as there is no authoritative guideline (afaik there is not), we might have

  • editor A that offers a text area to edit values
  • editor B (josm) that offers a text field (one line only) to edit values

Using editor A the result are newlines in the data and we cannot say with confidence that this is necessarily wrong (as we're missing a clear spec or at least a primitive consensus between developers on this).

My point is: By fixing it the way you have fixed it, you might (indirectly, when users do review and edit data) corrupt the data submitted by another editor. If there is a guideline you can give reference to, e.g. if that developer of the other editor comes to you complaining, then please ignore my point. But as long as this isn't the case, IMHO it is better to close this ticket using WON'T FIX than to mangle the data.

OP has a point in comment:4 that we can't just ignore: E.g. it might make more sense to offer a text area (at least for some keys!?) instead of a text field to supply data, it looks as though you might find a root cause of the OPs problem here:

If we are to say iD handles this wrong and should be fixed then we'd need a good reference to backup this argument. And if there is none, than maybe josm should be changed after all.

comment:10 Changed 2 months ago by stoecker

Resolution: fixed
Status: closedreopened

Actually allowing newline sounds sane. The other CTRL's should be forbidden thought.

comment:11 in reply to:  10 ; Changed 2 months ago by stoecker

Replying to stoecker:

Actually allowing newline sounds sane. The other CTRL's should be forbidden thought.

In display we can use U+2424 (␤)to show Newline.

comment:12 Changed 2 months ago by Don-vip

Ok. Please go ahead.

comment:13 in reply to:  11 Changed 2 months ago by cmuelle8

Replying to stoecker:

In display we can use U+2424 (␤)to show Newline.

What is the reason to prefer this over using a text area?

If you've looked at the iD ticket 1518 and the subsequent commit it looks as though they are enabling it for specific keys only. Newline in name or width tag does not make sense, does it?

Since the osm wiki is more or less documenting the data model, it may imho be best for all osm participants to use the Infoboxes there to additionally advocate if a field is expected to hold a single line of text only, or multiline. Currently, icons are used to visually indicate if a tag or key is applicable to nodes/ways/etc., so proposedly an additional icon symbolizing multiline text (slashed red in the single line case) could do.

Alternatively a "notes to software developers", or similar, section could be added to the Infobox in the osm wiki. "key" design choices may better be communicated at this central place than within this bug tracker or the one used to track issues for iD, not to forget about other editors.

Vespucci e.g. uses josm presets. So it might be a wise decision, if we are to introduce text areas, to drive this using the presets file.

comment:14 Changed 8 weeks ago by Don-vip

In 13246/josm:

see #15645 - revert r13226

comment:15 Changed 8 weeks ago by Don-vip

Milestone: 17.12

Modify Ticket

Change Properties
Set your email in Preferences
Action
as reopened The owner will remain team.
as The resolution will be set.
to The owner will be changed from team to the specified user.
The owner will change to naoliv
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.