Modify

Opened 4 years ago

Closed 3 weeks ago

Last modified 3 weeks ago

#11914 closed enhancement (fixed)

[Patch] OSM username in titlebar

Reported by: Allroads Owned by: team
Priority: normal Milestone: 20.01
Component: Core Version:
Keywords: Cc: tpikonen@…, Klumbumbus, Polarbear-j, stoecker, Don-vip

Description (last modified by Allroads)

I have two osm usernames, one special for a import, I get mixed up using them, because I do not see which authentication is used on the main screen.

Could this made be visible.

https://josm.openstreetmap.de/raw-attachment/ticket/11914/JOSM%20Username.PNG

Attachments (8)

JOSM Username.PNG (58.7 KB) - added by Allroads 4 years ago.
Example titlebar OSM username
11914.patch (1.3 KB) - added by GerdP 5 weeks ago.
It is easy to display the username but I don't know how to format it.
11914-v2.patch (1.3 KB) - added by GerdP 5 weeks ago.
use @
11914-v3.patch (4.9 KB) - added by GerdP 5 weeks ago.
- use tr(" ({0})", string)
11914-v4.patch (4.9 KB) - added by GerdP 5 weeks ago.
fix typo tile -> title
11914.2.patch (1010 bytes) - added by GerdP 3 weeks ago.
Solution or hack?
11914.3.patch (2.2 KB) - added by GerdP 3 weeks ago.
11914.4.patch (2.3 KB) - added by GerdP 3 weeks ago.
improve comments

Download all attachments as: .zip

Change History (46)

Changed 4 years ago by Allroads

Attachment: JOSM Username.PNG added

Example titlebar OSM username

comment:1 Changed 4 years ago by Allroads

Description: modified (diff)

comment:2 Changed 4 years ago by Don-vip

We could even implement something similar to Chrome's profile button:


http://i.stack.imgur.com/e7H6d.png

comment:3 Changed 4 years ago by simon04

When implementing the profile button, one could also take #2710 into account.

comment:4 Changed 3 years ago by simon04

Ticket #14106 has been marked as a duplicate of this ticket.

comment:5 Changed 3 years ago by simon04

Cc: tpikonen@… Klumbumbus added

comment:6 Changed 3 years ago by Allroads

Switch person is even better!

I like that.

comment:7 Changed 3 years ago by anonymous

While creating a new ticket, I was checking for potential open one. I found it :-)

JOSM main windows - osm multi user account management / transparency
Add/Show current logged in OSM username in title bar (or status bar)

I can't estimate about time consumption for this implementation. But if it's easier to put the name of the active OSM user to title bar, I would appreciate please start with this enhancement.

The mentioned comfortable user switch function could be integrated later (independently).

Thanks a lot

https://www2.pic-upload.de/img/33267876/k.png

comment:8 Changed 3 years ago by Klumbumbus

a bit related: #15013

comment:9 Changed 3 years ago by Polarbear-j

Cc: Polarbear-j added

Changed 5 weeks ago by GerdP

Attachment: 11914.patch added

It is easy to display the username but I don't know how to format it.

comment:10 Changed 5 weeks ago by GerdP

Summary: OSM username in titlebar[Patch] OSM username in titlebar

comment:11 Changed 5 weeks ago by simon04

Maybe follow the Twitter & Co @ syntax, such as Java OpenStreetMap Editor (@GerdP)?

comment:12 Changed 5 weeks ago by GerdP

Yes, looks better than the prefix "OSM username:"

Changed 5 weeks ago by GerdP

Attachment: 11914-v2.patch added

use @

comment:13 Changed 5 weeks ago by simon04

I favour the @ variant, but this is just my personal taste.

Some remarks:

  1. We use tr(" ({0})", string) in a few other places to allow addressing different spacing styles around parenthesis
  2. The title does not update after setting/changing the OSM user in the preferences. (Implementing this might be out of scope since UserIdentityManager cannot notify listeners at the moment.)

comment:14 Changed 5 weeks ago by GerdP

  1. Thanks for the hint, will use that
  2. Hmm, yes, the missing listener is not nice. I assumed that a mapper would have different configurations for his user names. In that case the user would not change within a session.

comment:15 Changed 5 weeks ago by stoecker

  1. Add a option to disable this in the prefs. I can think of situations where this is really unwanted (e.g. when making screenshots).
  2. Remember that username may be non-existing.

comment:16 in reply to:  15 ; Changed 5 weeks ago by GerdP

Replying to stoecker:

  1. Add a option to disable this in the prefs. I can think of situations where this is really unwanted (e.g. when making screenshots).

OK, good point. Should go into the "look and feel" dialog, right?

  1. Remember that username may be non-existing.

Does that mean that UserIdentityManager.getInstance() might return null?

comment:17 in reply to:  16 ; Changed 5 weeks ago by stoecker

Replying to GerdP:

Replying to stoecker:

  1. Add a option to disable this in the prefs. I can think of situations where this is really unwanted (e.g. when making screenshots).

OK, good point. Should go into the "look and feel" dialog, right?

I'd say so.

  1. Remember that username may be non-existing.

Does that mean that UserIdentityManager.getInstance() might return null?

Don't know. But I do know that username is optional. You can run josm completely without OSM and I e.g. also do this.

comment:18 in reply to:  17 Changed 5 weeks ago by GerdP

Replying to stoecker:

Replying to GerdP:

Replying to stoecker:

  1. Add a option to disable this in the prefs. I can think of situations where this is really unwanted (e.g. when making screenshots).

OK, good point. Should go into the "look and feel" dialog, right?

I'd say so.

OK, will add a checkbox "display user name in title" with default set to off.

  1. Remember that username may be non-existing.

Does that mean that UserIdentityManager.getInstance() might return null?

Don't know. But I do know that username is optional. You can run josm completely without OSM and I e.g. also do this.

OK, I think the null test already handles this but I'll make sure that it works. Might take a few days as I am busy with another project.

Changed 5 weeks ago by GerdP

Attachment: 11914-v3.patch added
  • use tr(" ({0})", string)

comment:19 Changed 5 weeks ago by GerdP

v3 also implements

  • add PreferenceChangedListener
  • add new checkbox in "look and feel" dialog (below the "Show splash screen at startup" checkbox)

I just noticed a typo. It should be "Show user name in title", not "Show user name in tile".
The listener calls refreshTitle if either the user or the preference draw.show-user was changed.

Changed 5 weeks ago by GerdP

Attachment: 11914-v4.patch added

fix typo tile -> title

comment:20 Changed 5 weeks ago by GerdP

Milestone: 20.01

comment:21 Changed 5 weeks ago by GerdP

Two thoughts:
1) the patch relies on the order in which listeners are notified. I am not sure if it is guaranteed that UserIdentityManager
is updated before MainFrame asked for the user name.
2) Maybe I should better evaluate the content of preference osm-server.username instead of calling UserIdentityManager.getInstance().getUserName()?

comment:22 Changed 5 weeks ago by GerdP

Hmm, this is quite complicated. I tested only the case that the username is changed in the preferences dialog.
If I got that right v4 will work fine in this case because UserIdentityManager is updated by org.openstreetmap.josm.io.auth.CredentialsManager.store() before any PreferenceChangeEvent is created.
What confused me was that UserIdentityManager also evaluates the PreferenceChangeEvent.

Still, there seem to be other ways to change the field UserIdentityManager.userName. For example, ChangesetQueryTask may show its own dialog to set user + password and directly updates UserIdentityManager without changing a preference.
This happens e.g. when you click "My changesets" in the "Changeset Management Dialog" in a fresh JOSM installation or if the given combination of user + password is not authenticated. Not sure why this is done.
I hope it is OK to ignore those special cases.

comment:23 Changed 4 weeks ago by simon04

Only listening to osm-server.username also neglects login via OAuth (which uses oauth.access-token.key).

Maybe the cleanest solution would be to maintain a ListenerList in UserIdentityManager, and register the listener in MainFrame on UserIdentityManager. Something like this:

  • src/org/openstreetmap/josm/data/UserIdentityManager.java

    a b  
    2121import org.openstreetmap.josm.spi.preferences.StringSetting;
    2222import org.openstreetmap.josm.tools.CheckParameterUtil;
    2323import org.openstreetmap.josm.tools.JosmRuntimeException;
     24import org.openstreetmap.josm.tools.ListenerList;
    2425import org.openstreetmap.josm.tools.Logging;
    2526
    2627/**
     
    5657public final class UserIdentityManager implements PreferenceChangedListener {
    5758
    5859    private static UserIdentityManager instance;
     60    private final ListenerList<UserIdentityListener> listeners = ListenerList.create();
    5961
    6062    /**
    6163     * Replies the unique instance of the JOSM user identity manager
    private UserIdentityManager() { 
    9698    public void setAnonymous() {
    9799        userName = null;
    98100        userInfo = null;
     101        fireUserIdentityChanged();
    99102    }
    100103
    101104    /**
    public void setPartiallyIdentified(String userName) { 
    114117                    MessageFormat.format("Expected non-empty value for parameter ''{0}'', got ''{1}''", "userName", userName));
    115118        this.userName = trimmedUserName;
    116119        userInfo = null;
     120        fireUserIdentityChanged();
    117121    }
    118122
    119123    /**
    public void setFullyIdentified(String userName, UserInfo userInfo) { 
    134138        CheckParameterUtil.ensureParameterNotNull(userInfo, "userInfo");
    135139        this.userName = trimmedUserName;
    136140        this.userInfo = userInfo;
     141        fireUserIdentityChanged();
    137142    }
    138143
    139144    /**
    public void preferenceChanged(PreferenceChangeEvent evt) { 
    311316            }
    312317        }
    313318    }
     319
     320    @FunctionalInterface
     321    public interface UserIdentityListener {
     322        void userIdentityChanged();
     323    }
     324
     325    public void addListener(UserIdentityListener listener) {
     326        listeners.addListener(listener);
     327    }
     328
     329    private void fireUserIdentityChanged() {
     330        listeners.fireEvent(UserIdentityListener::userIdentityChanged);
     331    }
    314332}
  • src/org/openstreetmap/josm/gui/MainFrame.java

    a b public void initialize() { 
    102104        // This listener is never removed, since the main frame exists forever.
    103105        MainApplication.getLayerManager().addActiveLayerChangeListener(e -> refreshTitle());
    104106        MainApplication.getLayerManager().addAndFireLayerChangeListener(new ManageLayerListeners());
     107        UserIdentityManager.getInstance().addListener(this::refreshTitle);
     108        Config.getPref().addKeyPreferenceChangeListener("draw.show-user", event -> this.refreshTitle());
    105109
    106110        refreshTitle();
    107111

comment:24 Changed 4 weeks ago by GerdP

Yes, thanks, much better :)

comment:25 Changed 4 weeks ago by GerdP

Resolution: fixed
Status: newclosed

In 15725/josm:

fix #11914: Allow to show user name in titlebar

  • implement UserIdentityListener in UserIdentityManager
  • add new checkbox "Show user name in title" in "Look and Feel" preference dialog
  • if enabled and not anonymous, show current user prefixed with "@" in titlebar of main window

comment:26 Changed 4 weeks ago by GerdP

Resolution: fixed
Status: closedreopened

It is possible to change the user stored in the CredentialsManager while uploading data.

comment:27 Changed 4 weeks ago by GerdP

Seems the approach was wrong. The user that is really in use may be stored in AbstractCredentialsAgent.memoryCredentialsCache.
I don't yet understand if this is an error or a feature. The user changes but UserIdentityManager is not aware of this.

Changed 3 weeks ago by GerdP

Attachment: 11914.2.patch added

Solution or hack?

comment:28 Changed 3 weeks ago by GerdP

It seems the value in AbstractCredentialsAgent.memoryCredentialsCache is not even changed when I use the preferences dialog to set a different user. This really looks like an error.

Changed 3 weeks ago by GerdP

Attachment: 11914.3.patch added

Changed 3 weeks ago by GerdP

Attachment: 11914.4.patch added

improve comments

comment:29 Changed 3 weeks ago by GerdP

Cc: stoecker Don-vip added
Priority: normalcritical

Patch 11914.4.patch fixes the problems mentioned in comment:27 and comment:29 as long as no plugin uses
CredentialsManager.registerCredentialsAgentFactory(CredentialsAgentFactory agentFactory)
Changes:

  • update UserIdentityManager when CredentialsManager is updated.
  • call purgeCredentialsCache() when user is changed so that we don't use previously user when the new user+password combination is bad.

@Dirk, Vincent:
I wonder why we allow a plugin to change such a basic security part. If I got that right every plugin can achive control here and send secret information to a third party? Maybe open a new ticket for this?

comment:30 Changed 3 weeks ago by Don-vip

Plugins can do anything. We do not have particular security layer restricting what they can do. Remember the "no more mapping" plugin.

comment:31 Changed 3 weeks ago by GerdP

Resolution: fixed
Status: reopenedclosed

In 15757/josm:

fix #11914: show OSM username in titlebar

  • update UserIdentityManager when CredentialsManager is updated.
  • call purgeCredentialsCache() when user is changed so that we don't use previously user when the new user+password combination is bad.

comment:32 Changed 3 weeks ago by Don-vip

Priority: criticalnormal

comment:33 Changed 3 weeks ago by GerdP

Just for clarification: I changed the status to critical because JOSM sometimes ignored a change of the user name made in the preference dialog. I was too lazy to open a new ticket for that. Probably not a good idea...

comment:34 Changed 3 weeks ago by Don-vip

Even that would not justify a critical issue to my eyes. Who really changes often its username? It's not a common scenario.

comment:35 Changed 3 weeks ago by GerdP

OK, it's unliky to happen but when it happens its effect may be that you upload a changeset with the wrong user. I consider this critical, esp. when JOSM shows that you work with user abc and in fact you upload as user xyz.

comment:36 Changed 3 weeks ago by stoecker

Well for JOSM the states are:

  • normal - all
  • major - more important issue affecting a group of users
  • critical - a large number of users is affected (we're getting a lot of duplicate reports), usually results in a hotfix and manual latest compilation
  • blocker - JOSM doesn't work at all for a majority of users

comment:37 Changed 3 weeks ago by GerdP

OK, thanks, I'll save a link to this list.

comment:38 Changed 3 weeks ago by Klumbumbus

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.