openstreetmap / iD

🆔 The easy-to-use OpenStreetMap editor in JavaScript.
https://www.openstreetmap.org/edit?editor=id
ISC License
3.37k stars 1.21k forks source link

Validator warning for syntactically invalid keys #5735

Open 1ec5 opened 5 years ago

1ec5 commented 5 years ago

If the user manually adds a key that’s obviously inconsistent with conventional key syntax, iD should issue a validator warning in the upload panel. For example, a key that contains an uppercase letter (other than FIXME), a space, an accented character, or :: is likely to be either a typo or an opportunity to suggest a more appropriate tag to an inexperienced mapper. Such issues may be caught by QA tools, but the upload panel is an opportunity to clarify the mapper’s intent so that other mappers don’t have to guess.

On that note, a simple validator warning message like “Invalid key” may not adequately explain the problem to the mapper. Perhaps an ℹ️ button could open this set of guidelines or a fuller message with tips on finding the appropriate tag to use.

There are some additional ideas around validating punctuation in osmlab/osmlint#234. There, the focus was on name tags, but for keys we can be much more strict without worrying about false positives.

bhousel commented 5 years ago

This sounds a lot like #4579 which you opened a while ago.

1ec5 commented 5 years ago

It does serve a similar purpose. This suggestion is purely about syntax, whereas #4579 would require hitting the taginfo or Wikibase API.

quincylvania commented 5 years ago

I'm all for this. It'd go great with #5830 since users would see the warning as soon as they enter the key.

matkoniecz commented 5 years ago

Note that tags may also contain (due to copy pasting) an invisible and unprintable characters - for example unicode bidi control ( see case for widespread bidi control character in https://josm.openstreetmap.de/ticket/17521 that caused https://github.com/openstreetmap/iD/issues/5895 ).

bhousel commented 5 years ago

Note that tags may also contain (due to copy pasting) an invisible and unprintable characters - for example unicode bidi control ( see case for widespread bidi control character in https://josm.openstreetmap.de/ticket/17521 that caused #5895 ).

I think iD already strips these out for things that look like URLs, however we wouldn't really know which out of all the millions of tags in OSM are allowed to contain these characters.

https://github.com/openstreetmap/iD/blob/d76baa9b8365f656c0996313fe13149d2fce2077/modules/util/clean_tags.js#L30-L39

pnorman commented 5 years ago

Checking tags would be fraught with problems, but keys should be safe.

quincylvania commented 5 years ago

I think we should do this under the "Invalid Formatting" validation rule added in #6494. We also have ℹ️ issue reference buttons now to explain specific syntax violations.