Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#15734 closed enhancement (fixed)

Icon and Shortcut checker warnings

Reported by: stoecker Owned by: team
Priority: normal Milestone: 18.02
Component: Core Version:
Keywords: Cc: Klumbumbus, Don-vip

Description (last modified by stoecker)

I updated our icon test script and it produced a handful of warnings. I fixed the scanning issues and here is the remaining stuff:

Hmm, on purpose I think

File logo exists twice as .svg and .png.

Should be fixed

ViewBox has float values for statusline/easting.svg: 0 0 57.207973 57.207977
ViewBox has float values for statusline/northing.svg: 0 0 57.207973 57.207977
ViewBox has float values for presets/craft/beekeeper.svg: 0 0 245.05 243.85
ViewBox has float values for presets/craft/painter.svg: 0 0 151.78 195.59
ViewBox has float values for presets/education/library.svg: 244.5 110 489 219.9
ViewBox has float values for presets/education/school.svg: 244.5 110 489 219.9
ViewBox has float values for presets/food/biergarten.svg: 0 0 453.72462 441.64626
ViewBox has float values for presets/food/pub.svg: 0 0 312.24864 359.23141
ViewBox has float values for presets/landmark/forest.svg: 0 0 1118.8397 1227.8107
ViewBox has float values for presets/landmark/reef.svg: 0 0 297.0498 213.31735
ViewBox has float values for presets/landmark/tree_row.svg: 0 0 970.83973 972.18771
ViewBox has float values for presets/landuse/farmland.svg: 0 0 553.9831 553.68453
ViewBox has float values for presets/landuse/meadow.svg: 0 0 253.87779 244.00716
ViewBox has float values for presets/landuse/orchard.svg: 0 0 670.93 802.78
ViewBox has float values for presets/landuse/vineyard.svg: 0 0 317.02 351.393
ViewBox has float values for presets/leisure/billboard.svg: 0 0 976.15625 729.85008
ViewBox has float values for presets/leisure/dogpark.svg: 244.5 110 489 219.9
ViewBox has float values for presets/leisure/fitness_station.svg: 0 0 495.00113 402.08099
ViewBox has float values for presets/leisure/theater.svg: 244.5 110 489 219.9
ViewBox has float values for presets/misc/contact.svg: 0 0 370.3 370.29
ViewBox has float values for presets/religion/hinduism.svg: -0.999 -0.762 242 201
ViewBox has float values for presets/shop/alcohol.svg: 0 0 254.44663 391.01157
ViewBox has float values for presets/shop/beverages.svg: 0 0 383.63842 391.01157
ViewBox has float values for presets/shop/body.svg: 0 0 485.2 493.06
ViewBox has float values for presets/shop/chemist.svg: 0 0 467.496 225.063
ViewBox has float values for presets/shop/clothes.svg: 244.5 110 489 219.9
ViewBox has float values for presets/shop/copyshop.svg: 244.5 110 489 219.9
ViewBox has float values for presets/shop/erotic.svg: 0 0 306 288.41
ViewBox has float values for presets/shop/hardware.svg: 244.5 110 489 219.9
ViewBox has float values for presets/shop/hearing_aids.svg: 244.5 110 489 219.9
ViewBox has float values for presets/shop/mall.svg: 0 0 208.532 270.512
ViewBox has float values for presets/shop/medical_supply.svg: 0 0 152.92334 167.08995
ViewBox has float values for presets/shop/music.svg: 0 0 432.28 432.28
ViewBox has float values for presets/shop/musical_instrument.svg: 244.5 110 489 219.9
ViewBox has float values for presets/shop/outdoor.svg: 0 0 218.184 265.487
ViewBox has float values for presets/shop/pet.svg: 244.5 110 489 219.9
ViewBox has float values for presets/shop/present.svg: 0 0 668.13 692.11
ViewBox has float values for presets/shop/tailor.svg: 244.5 110 489 219.9
ViewBox has float values for presets/shop/tattoo.svg: 0 0 581.8125 380.1875
ViewBox has float values for presets/shop/travel_agency.svg: 0 0 482.35 402.78
ViewBox has float values for presets/shop/wine.svg: 0 0 115.301 213.549
ViewBox has float values for presets/sightseeing/palaeontological_site.svg: 0 0 95.701 86.146
ViewBox has float values for presets/sport/billiards.svg: 0 0 598.14 598.14
ViewBox has float values for presets/sport/canoe.svg: 0 0 471.17099 413.276
ViewBox has float values for presets/sport/cycling.svg: 0 0 341.14413 298.03736
ViewBox has float values for presets/sport/golf.svg: 0 0 479.32132 510.47299
ViewBox has float values for presets/sport/miniature_golf.svg: 0 0 518.44158 510.47299
ViewBox has float values for presets/sport/racquetball.svg: 0 0 360.94283 366.95856
ViewBox has float values for presets/sport/running.svg: 0 0 289.64 289.64
ViewBox has float values for presets/sport/scuba_diving.svg: 0 0 100 83.101
ViewBox has float values for presets/sport/skiing.svg: 0 0 340.4894 378.98999
ViewBox has float values for presets/sport/soccer.svg: 0 0 105.66 100.619
ViewBox has float values for presets/sport/swimming.svg: 0 0 330.55416 250.32263
ViewBox has float values for presets/sport/tennis.svg: 0 0 381.20159 342.59891
ViewBox has float values for presets/misc/information/information.svg: 0 0 12.5915 12.5915
ViewBox has float values for presets/shop/groceries/bakery.svg: 244.5 110 489 219.9
ViewBox has float values for presets/shop/groceries/cheese.svg: 0 0 139.18741 104.05425
ViewBox has float values for presets/shop/groceries/seafood.svg: 244.5 110 489 219.9

These are commented as "to be created".

File gpx_segmented_track_to_tracks(.svg|.png) does not exist!
File gpx_split_tracks_to_layers(.svg|.png) does not exist!
File gpx_tracks_to_segmented_track(.svg|.png) does not exist!

These are unused I think. Didn't find a reference for them. Move to images_nodist?

gps-lines.png
layer-switcher-maximize.png
layer-switcher-minimize.png

Shortcut check complains about following (probably too many lines used for specifying the shortcut):

ERROR   src_plugins/OSMRecPlugin/src/org/openstreetmap/josm/plugins/osmrec/OSMRecPluginHelper.java#1728 Shortcut.registerShortcut(scKey, tr("Choose recent tag {0}", count), KeyEvent.VK_0+count, Shortcut.CTRL);final JosmAction action = new JosmAction(scKey, null, "Use this tag again", sc, false) {@Override
ERROR   src_plugins/OSMRecPlugin/src/org/openstreetmap/josm/plugins/osmrec/OSMRecPluginHelper.java#1738 Shortcut.registerShortcut(scsKey, tr("Apply recent tag {0}", count), KeyEvent.VK_0+count, Shortcut.CTRL_SHIFT);final JosmAction actionShift = new JosmAction(scsKey, null, "Use this tag again", scs, false) {@Override
ERROR   src_plugins/CustomizePublicTransportStop/src/ru/rodsoft/openstreetmap/josm/plugins/customizepublictransportstop/CustomizeStopAction.java#63     Shortcut.registerShortcut("tools:customizestop", tr("Tool: {0}", tr(CUSTOMIZE_STOP_ACTION_MENU_NAME)),KeyEvent.VK_U, Shortcut.DIRECT), true);action.putValue("help", ht("/Action/CustomizeStopAction"));
ERROR   src_plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/actions/EditHighlightedRelationsAction.java#34 Shortcut.registerShortcut("Edit Highlighted Relation", tr(actionName),KeyEvent.VK_K, Shortcut.ALT),false, "editHighlightedRelations", false);
ERROR   src_plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/actions/EdgeSelectionAction.java#46    Shortcut.registerShortcut("mapmode:edge_selection",tr("Mode: {0}", tr(MAP_MODE_NAME)),KeyEvent.VK_K, Shortcut.CTRL),
ERROR   src_plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/actions/AddStopPositionAction.java#59  Shortcut.registerShortcut("mapmode:stop_position",tr("Mode: {0}", tr(MAP_MODE_NAME)),KeyEvent.VK_K, Shortcut.CTRL_SHIFT),
ERROR   src_plugins/Mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/MapillaryDownloadViewAction.java#38  Shortcut.registerShortcut("Mapillary area", I18n.tr(DESCRIPTION),KeyEvent.VK_PERIOD, Shortcut.SHIFT
ERROR   src_plugins/Mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/MapillaryImportAction.java#31        * @see Shortcut#registerShortcut(String, String, int, int)*/public MapillaryImportAction() {
ERROR   src_plugins/Mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/MapillarySubmitCurrentChangesetAction.java#51        Shortcut.registerShortcut("Submit changeset to Mapillary", I18n."Submit the current changeset to Mapillary",KeyEvent.CHAR_UNDEFINED, Shortcut.NONE

Attachments (0)

Change History (50)

comment:1 by stoecker, 6 years ago

Cc: Don-vip added
Description: modified (diff)
Summary: Icon checker warningsIcon and Shortcut checker warnings
ERROR   src_plugins/OSMRecPlugin/src/org/openstreetmap/josm/plugins/osmrec/OSMRecPluginHelper.java#1728 Shortcut.registerShortcut(scKey, tr("Choose recent tag {0}", count), KeyEvent.VK_0+count, Shortcut.CTRL);final JosmAction action = new JosmAction(scKey, null, "Use this tag again", sc, false) {@Override
ERROR   src_plugins/OSMRecPlugin/src/org/openstreetmap/josm/plugins/osmrec/OSMRecPluginHelper.java#1738 Shortcut.registerShortcut(scsKey, tr("Apply recent tag {0}", count), KeyEvent.VK_0+count, Shortcut.CTRL_SHIFT);final JosmAction actionShift = new JosmAction(scsKey, null, "Use this tag again", scs, false) {@Override
ERROR   src_plugins/CustomizePublicTransportStop/src/ru/rodsoft/openstreetmap/josm/plugins/customizepublictransportstop/CustomizeStopAction.java#63     Shortcut.registerShortcut("tools:customizestop", tr("Tool: {0}", tr(CUSTOMIZE_STOP_ACTION_MENU_NAME)),KeyEvent.VK_U, Shortcut.DIRECT), true);action.putValue("help", ht("/Action/CustomizeStopAction"));
ERROR   src_plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/actions/EditHighlightedRelationsAction.java#34 Shortcut.registerShortcut("Edit Highlighted Relation", tr(actionName),KeyEvent.VK_K, Shortcut.ALT),false, "editHighlightedRelations", false);
ERROR   src_plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/actions/EdgeSelectionAction.java#46    Shortcut.registerShortcut("mapmode:edge_selection",tr("Mode: {0}", tr(MAP_MODE_NAME)),KeyEvent.VK_K, Shortcut.CTRL),
ERROR   src_plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/actions/AddStopPositionAction.java#59  Shortcut.registerShortcut("mapmode:stop_position",tr("Mode: {0}", tr(MAP_MODE_NAME)),KeyEvent.VK_K, Shortcut.CTRL_SHIFT),
ERROR   src_plugins/Mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/MapillaryDownloadViewAction.java#38  Shortcut.registerShortcut("Mapillary area", I18n.tr(DESCRIPTION),KeyEvent.VK_PERIOD, Shortcut.SHIFT
ERROR   src_plugins/Mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/MapillaryImportAction.java#31        * @see Shortcut#registerShortcut(String, String, int, int)*/public MapillaryImportAction() {
ERROR   src_plugins/Mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/MapillarySubmitCurrentChangesetAction.java#51        Shortcut.registerShortcut("Submit changeset to Mapillary", I18n."Submit the current changeset to Mapillary",KeyEvent.CHAR_UNDEFINED, Shortcut.NONE
ERROR   src_plugins/rex/src/org/openstreetmap/josm/plugins/rex/actions/TagRoundaboutAction.java#65      Shortcut.registerShortcut("menu:rex","Menu: Roundabout Expander",
ERROR   src_plugins/indoorhelper/src/controller/IndoorHelperController.java#113 Shortcut.registerShortcut("mapmode:space",tr(""), KeyEvent.VK_SPACE, Shortcut.DIRECT);this.SpaceAction = new SpaceAction();
ERROR   src_plugins/indoorhelper/src/controller/IndoorHelperController.java#118 Shortcut.registerShortcut("mapmode:ALT",tr(""), KeyEvent.VK_ENTER, Shortcut.DIRECT);this.EnterAction = new EnterAction();

comment:2 by stoecker, 6 years ago

See also table at end of DevelopersGuide/ShortcutsList.

comment:3 by stoecker, 6 years ago

@Vincent:

Probably the results of these 2 scripts should be tested in JOSM-Integration? The Shortcut result file is in "auto/" directory, the other can be started for the tests.

comment:4 by Don-vip, 6 years ago

Sure. What is the script, and how do I call it?

comment:5 by stoecker, 6 years ago

Description: modified (diff)

Sure. What is the script, and how do I call it?

  • geticons.pl in JOSM core ;-) Probably filtering the first line?
  • And the other one is: ERROR lines in shortcuts.txt in josm's "auto" directory

comment:6 by stoecker, 6 years ago

In 13276/josm:

see #15734 - remove -inkscape entry confusing the SVG optimizer

comment:7 by stoecker, 6 years ago

In 13277/josm:

see #15734 - fix icon detection

comment:8 by stoecker, 6 years ago

In 13278/josm:

see #15734 - allow shortcut parsing

comment:9 by Don-vip, 6 years ago

Warnings parsing is a powerful feature but requires a common logging format I can parse with a regex.
For reference, this is the information I give to Jenkins to create a Warning:

    Warning(String fileName, int lineNumber, String type, String category, String message);

I think it would be good to have something like:

<file>:<line>: <CATEGORY>: <Message>

(-1 if line is not applicable for a given warning)

comment:10 by stoecker, 6 years ago

Sorry. I don't understand that comment. Do you refer to the shortcuts or geticons.pl?

  • The icons script is easy adaptable, although no file and line can be provided and category is always warning I'd think. Each output is an warning.
  • For the shortcuts the format definition is ERROR\tfile#line\ttext, which can be directly converted to URL (see JOSM.py ShortcutsListMacro._getlink() for an example, but that's not necessary I think). Anything else is normal output and must be ignored.

comment:11 by stoecker, 6 years ago

In 13279/josm:

see #15734 - unify output of icon script to form name:message

comment:12 by Don-vip, 6 years ago

geticons.pl, I didn't look at shortcuts yet. Thanks!

comment:13 by stoecker, 6 years ago

In 13280/josm:

see #15734 - revert accidential checkin

comment:14 by stoecker, 6 years ago

In 13281/josm:

see #15734 - drop some old checks

comment:15 by stoecker, 6 years ago

In 13282/josm:

see #15734 - move icon script to scripts directory

comment:16 by stoecker, 6 years ago

Hmm, no warnings displayed in Jenkins... Still something wrong I assume.

comment:17 by Don-vip, 6 years ago

Yes the regex must be wrong

comment:18 by Don-vip, 6 years ago

OK it works for icons:
https://josm.openstreetmap.de/jenkins/job/JOSM-Integration/jdk=JDK8/1054/warnings4Result/

Did you fix all shortcuts warnings, or is my regex still failing?

in reply to:  18 comment:19 by stoecker, 6 years ago

Replying to Don-vip:

OK it works for icons:
https://josm.openstreetmap.de/jenkins/job/JOSM-Integration/jdk=JDK8/1054/warnings4Result/

Did you fix all shortcuts warnings, or is my regex still failing?

I don't touch the 9 remaining until the Jenkins job works ;-)

comment:20 by Don-vip, 6 years ago

Done :)

comment:21 by Klumbumbus, 6 years ago

In 13347/josm:

see #15734 - resize beekeeper icon

comment:22 by Klumbumbus, 6 years ago

Resizing the icons with float value viewboxes seems to be an unpleasant task. Similar to #12201 / [o31865] the icons are streched in some way which in the end in the example of the beekeeper resulted in the most sharp version of the icon within JOSM.

the old viewbox was 245.05 x 243.85
which is the same aspect ratio like 16 x 15,922 or 16,079 x 16

simply resizing to 16 x 16 resulted in a taller rendering size.
resizing to 16 x 15 resulted in an unsharp upper part of the icon
with resizing to 160 x 150 I get the exact same rendering result within JOSM.

Well, it doesn't have to be the exact same rendering result after resizing, but I think in most cases the icons are optimized for the stretching bug. So it seems to avoid loosing sharpness of the icons we need to precicely manually check every resized icon.

An hopefully the svgcleaner doesn't make things worse again ;)

comment:23 by Don-vip, 6 years ago

Milestone: 18.0118.02

comment:24 by Klumbumbus, 6 years ago

In 13362/josm:

see #15734 - resize painter icon, fix library and school icon code

comment:25 by stoecker, 6 years ago

Fixed shortcut parsing for remaining cases.

comment:26 by stoecker, 6 years ago

In 13369/josm:

see #15734 - fix two icons by simply cutting the size. The circle is no longer prefectly round, but this produces best looking results with josm :-), add statusline image resizing

comment:27 by stoecker, 6 years ago

Fixed 3 new Shortcut issues.

comment:28 by Klumbumbus, 6 years ago

In 13370/josm:

see #15734 - resize some icons to avoid float value view boxes

comment:29 by Klumbumbus, 6 years ago

In 13371/josm:

see #15734 - fix icon

comment:30 by Klumbumbus, 6 years ago

In 13372/josm:

see #15734 - fix icon

comment:31 by stoecker, 6 years ago

@Vincent:

For the 3 new (icon, shortcut, plugins) we get graphics in JOSM-Integration. Not for "ELI synchronization test". Would that be possible as well?

in reply to:  31 comment:32 by Don-vip, 6 years ago

Replying to stoecker:

we get graphics in JOSM-Integration. Not for "ELI synchronization test". Would that be possible as well?

Not without significant work. The underlying test system is not the same. Currently the ELI test counts as "1" in the list of failing unit tests, as for the IANA TLD test or the Taginfo "tags used more than 10000 times" test.

in reply to:  description comment:33 by Hb---, 6 years ago

Replying to stoecker:

... These are unused I think. Didn't find a reference for them. Move to images_nodist?

gps-lines.png Was used on Help/Menu/GPXLayer and Help/Action/GPXLayerCustomizeLineDrawing.

layer-switcher-maximize.png Was used in Help/Dialog/Minimap.

layer-switcher-minimize.png Seems not be in the Wiki.

comment:34 by Klumbumbus, 6 years ago

In 13381/josm:

see #15734 - resize some icons to avoid float value view boxes

comment:35 by Klumbumbus, 6 years ago

In 13406/josm:

see #15734 - resize some icons to avoid float value view boxes. Same or better display at 16px.

comment:36 by Klumbumbus, 6 years ago

In 13408/josm:

see #15734 - resize remaining icons to avoid float value view boxes. Same or better display at 16px.

comment:37 by Klumbumbus, 6 years ago

Only this warning is left:
logo: File exists twice as .svg and .png.

comment:38 by stoecker, 6 years ago

Fix it or add an ignore?

comment:39 by Don-vip, 6 years ago

It's used in JNLP file:

<icon href="https://josm.openstreetmap.de/svn/trunk/images/logo.png"/>

We cannot use SVG here: documentation states:

A URL pointing to the icon file, may be in one of the following formats: gif, jpg, png, ico.

comment:40 by Don-vip, 6 years ago

So we can probably move it in images_nodist/logo

Last edited 6 years ago by Don-vip (previous) (diff)

in reply to:  40 comment:41 by Klumbumbus, 6 years ago

Replying to Don-vip:

So we can probably move it in images_nodist/logo

Wouldn't this break all existing jnlp files on the users computers?

comment:42 by Don-vip, 6 years ago

I don't know. I think Java caches the icon, but there's only one way to be sure...

comment:43 by stoecker, 6 years ago

We should use "https://josm.openstreetmap.de/logo.png" in this case and add a redirect. We also can add an redirect for the old location.

comment:44 by Don-vip, 6 years ago

Good idea, can you please create the redirect? Then we can close this ticket :)

comment:45 by stoecker, 6 years ago

Resolution: fixed
Status: newclosed

In 13460/josm:

fix #15734 - Icon and Shortcut checker warnings

comment:46 by Klumbumbus, 6 years ago

Was there a workaround in the jenkins test as this didn't report the double logo icon for a while now?

comment:47 by Don-vip, 6 years ago

Yes I told Jenkins to ignore this specific warning. I guess I can remove this rule now.

comment:48 by Don-vip, 6 years ago

Done.

comment:49 by Klumbumbus, 6 years ago

Fine.

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.