Modify

Opened 2 years ago

Closed 2 years ago

Last modified 8 months ago

#21794 closed defect (fixed)

[PATCH] Allow for cases where tags can be URL or key values in Tag2Link

Reported by: aumpfbahn@… Owned by: team
Priority: normal Milestone: 22.06
Component: Core tag2link Version:
Keywords: Cc:

Description

Since the contact:facebook tag could be filled both using the full url or just the name, the Open facebook.com contextual menu doesn't handle the first ones properly.

If the tag is defined using the full url, the contextual menu doubles the hostname, resulting something like https://www.facebook.com/https://www.facebook.com/FacebookUserName.

So the issue lies in detecting if the tag contains an url, in order to build the link properly.

Attachments (2)

21794.patch (5.4 KB ) - added by taylor.smock 2 years ago.
Check tag value against url formatters, and if the tag value matches one of the url formatters ($1 -> .*), extract the group
21794.2.patch (8.0 KB ) - added by taylor.smock 2 years ago.
Add test

Download all attachments as: .zip

Change History (16)

comment:1 by gaben, 2 years ago

Component: CoreCore tag2link

comment:2 by taylor.smock, 2 years ago

This is interesting.

I'm working on a patch for this, but I think we should encourage people to use the contact:facebook=FacebookUserName over contact:facebook=https://www.facebook.com/FacebookUserName.

The reason for this is as follows:

  • Key:contact:facebook has the following formatter urls associated with it:
    • https://www.facebook.com/$1
    • https://www.messenger.com/t/$1
    • https://unavatar.now.sh/facebook/$1

The patch I'm working on goes through the formatter urls, replaces $1 with (.*), and extracts group 1 (in the example, FacebookUserName from the https:// value). This only works on exact matches. If someone adds https://facebook.com/SomeoneElse, it will not work as https://facebook.com/$1 is not in the formatter urls!

by taylor.smock, 2 years ago

Attachment: 21794.patch added

Check tag value against url formatters, and if the tag value matches one of the url formatters ($1 -> .*), extract the group

in reply to:  2 ; comment:3 by gaben, 2 years ago

Replying to taylor.smock:

I'm working on a patch for this, but I think we should encourage people to use the contact:facebook=FacebookUserName over contact:facebook=https://www.facebook.com/FacebookUserName.

+1 I would even add an autofix rule for this.

in reply to:  3 comment:4 by taylor.smock, 2 years ago

Replying to gaben:

+1 I would even add an autofix rule for this.

I've added a discussion section for encouraging FacebookUserName over https://www.facebook.com/FacebookUserName on Talk:Key:contact:facebook. Let us see where the community comes down on it before we add an autofix for it.

comment:5 by taylor.smock, 2 years ago

Summary: Open facebook links in the context menu[PATCH] Allow for cases where tags can be URL or key values in Tag2Link

by taylor.smock, 2 years ago

Attachment: 21794.2.patch added

Add test

comment:6 by gaben, 2 years ago

Sure, thanks! :)

The main OSM website needs support as well, as currently it is only processing URLs. If JOSM would do this, other tags can also benefit from the shorter profile ID type values.

Last edited 2 years ago by gaben (previous) (diff)

comment:7 by anonymous, 2 years ago

I would like to add that I pointed facebook since is the most common key; but this issue also happens in twitter and instagram (that I've tested). Any contact:whatever key is quite possible being affected in a similar way

comment:8 by taylor.smock, 2 years ago

@anonymous: I've fixed it for general cases with attachment:21794.2.patch . But only if the URL matches one of the Tag2Link URL formatters.

However, since I was looking at the contact:facebook example, and Facebook has multiple formatters (of note, one for FB Messenger and one for FB itself), I figured it would be a good idea to kick off a discussion about preferring the profile over the url.

@gaben: Just get people involved in the discussion. Either for or against. The tag has been used since 2014, so I don't want to make any changes without community discussion and support.

comment:9 by gaben, 2 years ago

I know, just collecting things for consideration.

comment:10 by taylor.smock, 2 years ago

Resolution: fixed
Status: newclosed

In 18475/josm:

Fix #21794: Allow for cases where tags can be URL or key values in Tag2Link

This also adds a Pattern cache (memory only) to avoid many allocations for
the same pattern in Tag2Link.

comment:11 by gaben, 2 years ago

What about the autofix rule I came up with? Should I create a new ticket?

comment:12 by taylor.smock, 2 years ago

Definately a new ticket. It is (largely) unrelated to this one, with the caveat that it was what discovered this problem in the first place.

comment:13 by skyper, 2 years ago

Milestone: 22.06

in reply to:  12 comment:14 by gaben, 8 months ago

Replying to taylor.smock:

Definately a new ticket. It is (largely) unrelated to this one, with the caveat that it was what discovered this problem in the first place.

See #23244.

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.