Skip to content

Mentions: Standardize around only displaying username #1510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 12 commits into from
Mar 31, 2025
Merged

Mentions: Standardize around only displaying username #1510

merged 12 commits into from
Mar 31, 2025

Conversation

@obenland
Copy link
Member

@obenland obenland commented Mar 25, 2025 •
edited
Loading

Fixes #1499.

Proposed changes:

  • Removes host from webfinger to create a username mention in comments.

Other information:

  • Have you written new tests for your changes, if applicable?

Testing instructions:

  • On a post with comment from the Fediverse: Reply to that comment and mention the comment author.
  • Make sure the mention is username-only.

Changelog entry

  • Automatically create a changelog entry from the details below.
Changelog Entry Details

Significance

  • Patch
  • Minor
  • Major

Type

  • Added - for new features
  • Changed - for changes in existing functionality
  • Deprecated - for soon-to-be removed features
  • Removed - for now removed features
  • Fixed - for any bug fixes
  • Security - in case of vulnerabilities

Message

Standardized mentions to use usernames only.

@obenland obenland requested review from a team and jeherve March 25, 2025 18:23
@obenland obenland self-assigned this Mar 25, 2025
pfefferle
pfefferle reviewed Mar 26, 2025
View reviewed changes
@jeherve
Copy link
Member

jeherve commented Mar 26, 2025

genuinely don't know whether this is the best path forward, looking for feedback on the approach.

Looking at how Mastodon does things, wouldn't it be best to use the username only, instead of the full account name?

e.g. your mention here is displayed as @evan, not @evan@cosocial.ca:

image

@obenland
Copy link
Member Author

obenland commented Mar 26, 2025

Ha, sure! I had a 50:50 shot 😄

@obenland
Copy link
Member Author

obenland commented Mar 26, 2025

The only downside I can think of is when you're responding to multiple users with the same username, like @index@ghost.org and @index@activitypub.ghost.org.

@jeherve
Copy link
Member

jeherve commented Mar 26, 2025

The only downside I can think of is when you're responding to multiple users with the same username, like @index@ghost.org and @index@activitypub.ghost.org.

That's a good point. Do you know how Mastodon handles this? Do they detect that use-case?

@pfefferle
Copy link
Member

pfefferle commented Mar 26, 2025

Mastodon has Gravatar like pop-overs, that show details about the person.

Screenshot 2025-03-26 at 16 29 59

@jeherve
Copy link
Member

jeherve commented Mar 26, 2025

Mastodon has Gravatar like pop-overs, that show details about the person.

Good point. And I see my desktop app (Semaphore) does have a title attribute on the link that shows the full handle. Elk, however, has nothing.

@pfefferle
Copy link
Member

pfefferle commented Mar 26, 2025

I think having a title attribute would bridge the gap here!?!

@obenland obenland requested review from a team and pfefferle March 27, 2025 14:49
@mattwiebe mattwiebe mentioned this pull request Mar 28, 2025
Merged
1 task
@pfefferle
Copy link
Member

pfefferle commented Mar 31, 2025

I think this is fine but still: #1510 (review)

@obenland
Copy link
Member Author

obenland commented Mar 31, 2025

Is the comment you linked to not solved?

@pfefferle
Copy link
Member

pfefferle commented Mar 31, 2025

It looked like, but it seems that it was somehow reverted... maybe because of a merge conflict... or the GitHub UI does not show it properly... I can see only changes in three files and the Mention Class is not one of them 🤔

@obenland
Copy link
Member Author

obenland commented Mar 31, 2025

Right, that's the change from full account names to only usernames, as requested in that comment. There are no changes necessary in the Mentions class.

Let me update the PR description to reflect the change of direction.

@obenland obenland changed the title Mentions: Use full account name with preferredUsername Mentions: Standardize around only displaying username Mar 31, 2025
@pfefferle
Copy link
Member

pfefferle commented Mar 31, 2025

But that is what I meant, both implementations produce different HTML and I would love to have it normalized! See the <span />.

@obenland
Copy link
Member Author

obenland commented Mar 31, 2025 •
edited
Loading

But that is what I meant, both implementations produce different HTML and I would love to have it normalized! See the .

Fixed in 43636dd.

mattwiebe
mattwiebe approved these changes Mar 31, 2025
View reviewed changes
Copy link
Contributor

@mattwiebe mattwiebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests well, nice change 👍🏻

@obenland obenland merged commit d14cbfe into trunk Mar 31, 2025
11 checks passed
@obenland obenland deleted the fix/mentions branch March 31, 2025 18:33
jsit added a commit to jsit/wordpress-activitypub that referenced this pull request Nov 1, 2025
@jsit
Merge tag '5.6.1' into fix/nicename
0812126 
* tag '5.6.1': (47 commits)
  Release 5.6.1 (Automattic#1542)
  Fix: Use specified time formatting for Outbox Activities (Automattic#1537)
  Fix: Default value for empty `movedTo` field (Automattic#1539)
  Fix: Post Interactions settings (Automattic#1540)
  Release 5.6.0 (Automattic#1534)
  lover significance (Automattic#1533)
  Scheduler: Don't federate Deletes of unfederated posts. (Automattic#1528)
  Move: use `$from` for object in `Move::externally` (Automattic#1531)
  Fix: Properly traverse the object hierarchy to get the ID (Automattic#1518)
  Mentions: Standardize around only displaying username (Automattic#1510)
  Tests: Reduce chance of time offsets (Automattic#1527)
  Import: Don't federate new attachments (Automattic#1526)
  Importer: Format import count number. (Automattic#1525)
  Move: Add a Mastodon importer (Automattic#1502)
  Move: use `$from` for object in `Move::internally` (Automattic#1516)
  Fix: Do not send self-replies (Automattic#1517)
  Fix: Delete Undone comments instead of trashing them (Automattic#1520)
  Dispatcher: Adhere to passed batch size. (Automattic#1514)
  Add interaction settings (Automattic#1395)
  Fix: Set proper default, to show Welcome screen by default (Automattic#1512)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve Awaiting requested review from jeherve

@pfefferle pfefferle Awaiting requested review from pfefferle

1 more reviewer

@mattwiebe mattwiebe mattwiebe approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@obenland obenland

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Mentions: inconsistent ActivityPub and site representation

6 participants

@obenland @jeherve @pfefferle @mattwiebe @matticbot