Modify

Opened 8 years ago

Closed 7 years ago

#7027 closed defect (fixed)

Windows user preferences are lost when running 4550 after 4553

Reported by: Don-vip Owned by: team
Priority: critical Milestone:
Component: Core Version: tested
Keywords: preferences Cc:

Description (last modified by Don-vip)

Damn, I just had the following error message after upgrading to 4550:


I don't know what the error was, because no backup file has been created, despite the popup saying me it would.

The only two preferences files located in my %APPDATA%/JOSM are "preferences" and "preferences_backup", almost equal each other (only one line "gui.geometry=1920x1034+0+0" present in preferences and not in preferences_backup).

The file "preferences.bak" I was expecting to find is not there. I don't know if it has been deleted or never created.

Any idea on this ? This could be a major problem if the new tested version drops all user preferences. Maybe it is just located somewhere else ?

Attachments (4)

screenshot.png (67.8 KB) - added by Don-vip 8 years ago.
preferences.xml.patch (5.7 KB) - added by bastiK 8 years ago.
preferences.xml2.patch (7.3 KB) - added by bastiK 8 years ago.
improved patch
preferences-structures.patch (38.2 KB) - added by bastiK 8 years ago.

Download all attachments as: .zip

Change History (47)

Changed 8 years ago by Don-vip

Attachment: screenshot.png added

comment:1 Changed 8 years ago by Don-vip

Description: modified (diff)

comment:2 Changed 8 years ago by Don-vip

I see r4553 switched preferences file format to XML. I think I've run JOSM once from my up-to-date Eclipse, then launched r4550. That would explain all these errors and the huge number of lines for a preferences file.

comment:3 Changed 8 years ago by Don-vip

Confirmed. Launch 4553, then 4550, the preferences file disappears. Shouldn't we release a new tested version containing read-only XML support before effectively switch to XML saving ? I think it's a common practice to switch between 2 versions of JOSM (at least tested/latest), and this change is going to break many user configurations without any warning.

comment:4 Changed 8 years ago by Don-vip

It appears to be a Windows issue. In debug mode, I can see File.rename() of "C:\Users\Vincent\AppData\Roaming\JOSM\preferences" to "C:\Users\Vincent\AppData\Roaming\JOSM\preferences.bak" fails under Windows, I think we should change the filename to something else, "preferences_invalid" or something like that. In all cases, we need to check the return value and do something it the operation fails. The proper way seems to abort JOSM startup, no ?

comment:5 Changed 8 years ago by Don-vip

Summary: User preferences can be lostWindows user preferences can be lost when running 4550 after 4553

comment:6 Changed 8 years ago by Don-vip

Priority: majorcritical
Summary: Windows user preferences can be lost when running 4550 after 4553Windows user preferences are lost when running 4550 after 4553

comment:7 Changed 8 years ago by stoecker

Is downgrading really a common issue?

comment:8 Changed 8 years ago by Don-vip

Users that encounter a regression on JOSM latest may want to use tested version. Plugin developers may want to check if their plugin works well with both the tested and latest version. Personnaly, I frequently launch older JOSM versions in order to reproduce user bugs. I will no longer do that for versions older than the one that fixes this issue, it's a real pain to lose all my preferences.

comment:9 Changed 8 years ago by stoecker

Resolution: fixed
Status: newclosed

In [4554/josm]:

temporary disable XML saving till next tested is released - fix #7027

comment:10 Changed 8 years ago by stoecker

4554 saves old style preferences again (except prefs are already loaded in XML format). Will be switched again in December after next tested.

comment:11 Changed 8 years ago by stoecker

In [4555/josm]:

save XML for new files always - see #7027

comment:12 Changed 8 years ago by Don-vip

In [4564/josm]:

fix #7027 - Fixes the two reasons why rename fail under Windows (handle kept open and fails if backup already exist)

comment:13 Changed 8 years ago by bastiK

Resolution: fixed
Status: closedreopened

Why not call it "preferences.xml"? This file extension is common convention and it avoids the compatibility hacks.

comment:14 Changed 8 years ago by bastiK

The xml format is not fully optimized for our data structures. Instead of

 <collection key='imagery.entries.1'>
  <entry value='name:LandSat' />
  <entry value='type:wms' />
  <entry value='url:http://irs.gis-lab.info/?layers=landsat&amp;SRS={proj}&amp;WIDTH={width}&amp;HEIGHT={height}&amp;BBOX={bbox}' />
 </collection>
 <collection key='imagery.entries.2'>
  <entry value='name:MapQuest Open Aerial' />
  <entry value='type:tms' />
  <entry value='url:http://oatile{switch:1,2,3,4}.mqcdn.com/tiles/1.0.0/sat/{zoom}/{x}/{y}.png' />
 </collection>

I would expect something like this:

<collection key='imagery.entries'>
 <entry>
  <tag key='name' value='LandSat'/>
  <tag key='type' value='wms'/>
  <tag key='url' value='http://irs.gis-lab.info/?layers=landsat&amp;SRS={proj}&amp;WIDTH={width}&amp;HEIGHT={height}&amp;BBOX={bbox}'/>
 </entry>
 <entry>
  <tag key='name' value='MapQuest Open Aerial'/>
  <tag key='type' value='tms'/>
  <tag key='url' value='http://oatile{switch:1,2,3,4}.mqcdn.com/tiles/1.0.0/sat/{zoom}/{x}/{y}.png'/>
 </entry>
</collection>

I suppose we can fix this step by step?

Last edited 8 years ago by bastiK (previous) (diff)

comment:15 Changed 8 years ago by stoecker

You're right. The newer structures you introduced aren't yet implemented in XML.

Changed 8 years ago by bastiK

Attachment: preferences.xml.patch added

comment:16 Changed 8 years ago by bastiK

This patch writes xml to preferences.xml and just keeps the old preference file as it is.

comment:17 in reply to:  15 ; Changed 8 years ago by bastiK

Replying to stoecker:

You're right. The newer structures you introduced aren't yet implemented in XML.

Your Array type deserves special handling as well.

Changed 8 years ago by bastiK

Attachment: preferences.xml2.patch added

improved patch

comment:18 in reply to:  17 ; Changed 8 years ago by stoecker

Replying to bastiK:

Your Array type deserves special handling as well.

XML code is there for years. I thought that we never switch, when I wait again until the format is perfect. So I decided to start now and leave improvements to the future :-)

We need to change the internal prefs handling a bit as well, as currently we do not really know which prefs key is of which type.

I see no problems with your patch, why didn't you apply it directly?

comment:19 in reply to:  18 Changed 8 years ago by bastiK

Replying to stoecker:

I see no problems with your patch, why didn't you apply it directly?

I don't want to make matters worse and give you and others time to object.

comment:20 Changed 8 years ago by bastiK

A bit off topic, but here are some things we can change in the process:

  • put(key, "") should not remove the tag, but map the key to the empty string
  • likewise, allow saving collections of size zero as values
  • put(key, null) should not remove the tag, but throw an exception (?)

Rename the structures to match the proposed new return types List<String>, List<List<String>> and List<Map<String,String>>:

  • getCollection -> getList
  • getArray -> getListList
  • getListOfStructs -> getMapList

comment:21 Changed 8 years ago by bastiK

In [4592/josm]:

see #7027 - xml formatted preference data is now written to the new file preferences.xml. The old preference file is read when no xml file is found, but otherwise not touched from now on.

comment:22 Changed 8 years ago by stoecker

Why do you want to change semantics of put()? What improvement would this bring?

comment:23 in reply to:  22 Changed 8 years ago by bastiK

Replying to stoecker:

Why do you want to change semantics of put()? What improvement would this bring?

E.g. print plugin adds a label to the map:

String text = Main.pref.get("print.attribution", "OSM Map data (c) OpenStreetMap contributors, CC-BY-SA");

If you don't want that label, you could go to advanced preferences and set the property "print.attribution" to the empty string. This doesn't work, because then the tag is removed and the default value is used again.

Obviously there are workarounds (e.g. use a single space character) but that's silly.

comment:24 Changed 8 years ago by stoecker

I fear that such a change would break a lot of code. Most places silently assume the current behaviour.

Changed 8 years ago by bastiK

comment:25 Changed 8 years ago by bastiK

This patch adds internal data structures that match the types of collection, array and listOfStructs. It should work mostly. What's missing is refactoring of PreferenceChangeEvent and a GUI to edit these values in advanced preferences pane. I'd suggest to postpone the GUI and check in what's done.

comment:26 in reply to:  24 Changed 8 years ago by bastiK

Replying to stoecker:

I fear that such a change would break a lot of code. Most places silently assume the current behaviour.

Yes, I know. If it's too much work, I'll drop it, but it has been bugging me for some time now. :)

comment:27 Changed 8 years ago by bastiK

In [4612/josm]:

see #7027 - internal structures for preferences (expect some problems next few days)

comment:28 Changed 8 years ago by bastiK

TODO:

  • Status report (uses old file)
  • Advanced preferences (shows only simple key=value prefs)
  • Windows installer

comment:29 Changed 8 years ago by bastiK

In [4634/josm]:

advanced preference: dialogs to edit complex preference settings (list, list of lists, list of maps) (see #7027)

comment:30 Changed 8 years ago by bastiK

In [4635/josm]:

status report should use new xml preference file (see #7027)

comment:31 Changed 8 years ago by bastiK

The file josm.nsi should be changed, to write to preference.xml instead of preference. I'm not familiar with the scripting language, so if anyone has a clue, please go ahead.

comment:32 Changed 8 years ago by bastiK

In [4656/josm]:

Fix ambiguities of empty collection vs. empty array by renaming the top level xml tags. This change breaks forward compatibility. (see #7027)

comment:33 Changed 8 years ago by stoecker

I fixed the installer in [o27235]. Anyone wants to test it download/windows/josm-setup-4661.exe?

comment:34 in reply to:  33 ; Changed 8 years ago by bastiK

Replying to stoecker:

I fixed the installer in [o27235]. Anyone wants to test it download/windows/josm-setup-4661.exe?

You can select certain plugins in the installer dialog. This should be respected by the plugins=... pref entry.

comment:35 in reply to:  34 ; Changed 8 years ago by stoecker

You can select certain plugins in the installer dialog. This should be respected by the plugins=... pref entry.

Actually I have no idea how to do this. I don't use Windows very often.

comment:36 in reply to:  35 Changed 8 years ago by bastiK

Replying to stoecker:

You can select certain plugins in the installer dialog. This should be respected by the plugins=... pref entry.

Actually I have no idea how to do this. I don't use Windows very often.

Enter

wine josm-setup-4661.exe

a dialog pops up > ok > next > accept

Then you can select components, e.g. plugins.

Afterwards, preference file can be found in ~/.wine/.../JOSM.

comment:37 Changed 8 years ago by Don-vip

I've run it on Win 7 x64. Some remarks/questions:

  • The install seems to work fine (a new file preferences.xml is created when no previous preferences exist, an existing preferences file is not deleted) but the launched version is r4550, is it expected or not ?
  • I'll add a new file for the French language, if you're OK
  • The authors list is not up to date
  • I think we should at least remove the "WMS downloader" plugin (is it the old WMS plugin ?) and add the license change plugin to default suggested plugins. Maybe others, based on the most popular plugins we see in bug reports ? (Off-topic: By the way, do we have any kind of statistics on plugins usage ?)

comment:38 in reply to:  37 Changed 8 years ago by bastiK

Replying to Don-vip:

  • I think we should at least remove the "WMS downloader" plugin (is it the old WMS plugin ?)

I think "WMS downloader" is webkit-image-qt for Yahoo imagery. It's now obsolete, but may be needed again in future (?).

and add the license change plugin to default suggested plugins. Maybe others, based on the most popular plugins we see in bug reports ? (Off-topic: By the way, do we have any kind of statistics on plugins usage ?)

Yes, I can send you the stats by email.

comment:39 in reply to:  37 ; Changed 8 years ago by stoecker

Actually I have no idea how to do this. I don't use Windows very often.

... do this IN THE INSTALLER. :-)

  • The install seems to work fine (a new file preferences.xml is created when no previous preferences exist, an existing preferences file is not deleted) but the launched version is r4550, is it expected or not ?

Not expected, but understandable. I called only a part of the build script and probably missed something.

  • I'll add a new file for the French language, if you're OK

Sure.

  • The authors list is not up to date

You mean the one in JOSM svn. Update it when necessary.

  • I think we should at least remove the "WMS downloader" plugin (is it the old WMS plugin ?) and add the license change plugin to default suggested plugins. Maybe others, based on the most popular plugins we see in bug reports ? (Off-topic: By the way, do we have any kind of statistics on plugins usage ?)

I would not remove it. It is not really large and does no harm.

comment:40 in reply to:  39 ; Changed 8 years ago by Don-vip

Replying to stoecker:

  • I'll add a new file for the French language, if you're OK

Sure.

Done in [o27246] :)

  • The authors list is not up to date

You mean the one in JOSM svn. Update it when necessary.

No, it's not this one. I don't understand where this list comes from, but it's not the one used in JOSM itself. It's really outdated as it ends by "The current JOSM maintainer is Frederik Ramm <frederik@…>." ;)

comment:41 in reply to:  40 Changed 8 years ago by stoecker

You mean the one in JOSM svn. Update it when necessary.

No, it's not this one. I don't understand where this list comes from, but it's not the one used in JOSM itself. It's really outdated as it ends by "The current JOSM maintainer is Frederik Ramm <frederik@…>." ;)

Found and fixed.

comment:42 Changed 8 years ago by bastiK

In [o27251] - fix nsis to install only the selected plugins

comment:43 Changed 7 years ago by stoecker

Resolution: fixed
Status: reopenedclosed

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.