Page MenuHomePhabricator

[lib] Modify markdownUserMentionRegex to match ENS names as well
ClosedPublic

Authored by rohan on Dec 18 2023, 11:21 AM.
Tags
None
Referenced Files
F3506177: D10388.id34999.diff
Fri, Dec 20, 3:44 PM
F3505260: D10388.diff
Fri, Dec 20, 12:55 PM
Unknown Object (File)
Wed, Nov 27, 3:00 AM
Unknown Object (File)
Nov 8 2024, 10:02 PM
Unknown Object (File)
Nov 8 2024, 7:17 PM
Unknown Object (File)
Nov 8 2024, 7:17 PM
Unknown Object (File)
Nov 8 2024, 7:17 PM
Unknown Object (File)
Nov 8 2024, 1:13 PM
Subscribers

Details

Summary

Currently, for @ mentions, we use the oldValidUsernameRegexString to detect a valid account name ([a-zA-Z0-9-_]+). Since we're now extending support to @ mention users with ENS names, names can have . in them, so we need to adjust the regex accordingly.

Rather than modifying the oldValidUsernameRegexString which is used for account validation, I stopped using oldValidUsernameRegexString when detecting @ mentions and made a separate regex string. It is pretty close to the old one, except it also adds support for .'s in names

Depends on D10387

Test Plan

Put some log statements in matchUserMentions and made sure that the line const username = result[2]; correctly extracted ENS names.

Previously it would only match jack, but now it can match jack.eth

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Looking at usages of oldValidUsernameRegexString I can see that it is used in oldValidUsernameRegex, which is used for something mention related on the keyserver. Should that be changed?

lib/shared/mention-utils.js
51 ↗(On Diff #34807)

I'd rather this constant be defined in account-utils, next to all other "ValidUsernameRegexString"s, with a comment explaining what it is.

In D10388#301357, @inka wrote:

Looking at usages of oldValidUsernameRegexString I can see that it is used in oldValidUsernameRegex, which is used for something mention related on the keyserver. Should that be changed?

That should actually be fine as-is because we run the regex against the raw username (so either something like jack or 0x89374e329419043028, not the resolved ENS name). We want to make sure the raw username is valid, and that's why I kept it as oldValidUsernameRegex.

The regex in this diff is to match resolved ENS names to bold them, so that's why I used one that includes the . character

Move constant to account-utils.js with a comment

Thank you for explaining!

This revision is now accepted and ready to land.Dec 22 2023, 12:55 AM