Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#9462 closed defect (fixed)

fix potentially invalid opening hours in presets

Reported by: aceman Owned by: team
Priority: normal Milestone: 14.01
Component: Internal preset Version: tested
Keywords: Cc:

Description

Some default presets contain suggestions for opening hours or service times, like this:
<combo key="service_times" text="Service Times" delimiter="|" values="18:00|sunset,sunrise|Su 9:30,11:00|Sa,Su,PH 9:00|Sa 18:00; Su 10:45|Fr 08:00-18:00; Apr 10-15 off; Jun 07:00-20:00; Aug off; Dec 24 08:00-24:00|Sa 10:00+|week 1-53/2 Fr 09:00-12:00; week 2-52/2 We 09:00-12:00" />

They include times like "9:00", which seem to be invalid per the wiki at http://wiki.openstreetmap.org/wiki/Key:opening_hours :
"hh hour, always two digits number in 24h basis (no am/pm), in the format "hh:mm" · (e.g., > Fr 08:30-20:00) "

The new internal JOSM validator seems to accept such times without warning. However, e.g. the OpeningHoursEditor plugin chokes on them.

So if those values are really invalid, the presets need to be fixed and also the validator to reject such syntax if found in a tag.

Attachments (0)

Change History (10)

comment:1 Changed 6 years ago by simon04

For validating the opening_hours we use https://github.com/ypid/opening_hours.js. Thus any required modification must be requested there.

I vote for allowing 9:00 since the opening_hours syntax shouldn't be more restrictive than necessary and in this case there's no ambiguity.

comment:2 Changed 6 years ago by mkoniecz

In this case I think that changing wiki is preferable, values are already extremely ugly, there is no need to make it even worse.

comment:3 Changed 6 years ago by aceman

It is probably easier to parse the string when the hours are always 2 digits. It is already complicated enough.

comment:4 Changed 6 years ago by Don-vip

Resolution: wontfix
Status: newclosed

+1 with Simon, these values are not ambiguous and the library allows them, so it's ok to do nothing in core. The openinghourseditor plugin needs however to rely now on the library we embed to parse opening_hours values instead of defining its own, limited, JavaCC parser. It is needed anyway to achieve #8913 (and probably a lot of other tickets concerning this plugin). I don't know if it's easily feasible now, there's maybe something to ask to ypid on Github to help us to do that (I haven't really looked in detail if we can get "decoded fragments" we could then display in the graphic component or if we need to create a new issue).

comment:5 Changed 6 years ago by simon04

In 6533/josm:

see #9462 - prettify valid opening_hours values as OTHER test error fixes

comment:6 Changed 6 years ago by aceman

This is not a question of ambiguity but following what the spec says and what other apps may expect.
I don't think we should promote out-of-spec values before it is discussed/defined on the wiki.

I do not understand what the patch does. It seems to only update some tests of the syntax. And it seems to use some .prettifiedValue of opening hours.

Can you please state what value is saved onto the server if I set opening_hours=9:00 ? Is the value changed to 09:00 or is it sent as is into the changeset?

comment:7 Changed 6 years ago by simon04

r6533 provides the prettified value as "other error" if no real warning is found.

We currently do not automatically change any opening_hours in the course of the upload.

I agree that the values in the should not necessarily need to be prettified – I'll look into that.

comment:8 Changed 6 years ago by simon04

Resolution: wontfixfixed

In 6568/josm:

fix #9462 - fix preset values for opening_hour/… such that the OpeningHourTest does not yield warnings, add unit test that validates all preset values

comment:9 Changed 6 years ago by aceman

You may silently accept single digit hours internally if you really want, but at least do not advertise them in the default presets. This patch seems to update the service times to 2 digit format that I wanted. Thanks for the fix.

comment:10 Changed 6 years ago by Don-vip

Milestone: 14.01

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.

Add Comment


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

 
Note: See TracTickets for help on using tickets.