Skip to content
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.

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

Count all URLs in text as 23 characters flat, do not count domain part of usernames #4427

Merged
merged 2 commits into from Jul 28, 2017
Merged

Count all URLs in text as 23 characters flat, do not count domain part of usernames #4427

merged 2 commits into from Jul 28, 2017

Conversation

Gargron
Copy link
Member

@Gargron Gargron commented Jul 28, 2017 •
edited

TODO:

  • Backend validation adjusted
  • Tests
  • Web UI counter adjusted

@Gargron Gargron added the work in progress Not to be merged, currently being worked on label Jul 28, 2017
@Gargron Gargron removed the work in progress Not to be merged, currently being worked on label Jul 28, 2017
nightpool
nightpool reviewed Jul 28, 2017
View reviewed changes
app/javascript/mastodon/features/compose/util/counter.js

export function countableText(inputText) {
return inputText
.replace(/https?:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,6}\b([-a-zA-Z0-9@:%_\+.~#?&//=]*)/g, urlPlaceholder)
Copy link
Member

Choose a reason for hiding this comment

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

why 256 instead of {2,}?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did copy-paste the regex. If you have a more authoritative regex, be my guest

Copy link
Contributor

@progval progval Jul 29, 2017 •
edited

Choose a reason for hiding this comment

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

The 256 part is not a big issue, as all domain names are limited to 255 characters in practice.
However, the regexp for the TLD is [a-z]{2,6}, which is wrong because TLDs are not limited to 6 characters. eg. .network (used by mastodon.network)

Copy link

Choose a reason for hiding this comment

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

Fixed by #4463.

@Gargron Gargron merged commit 53b2b1b into master Jul 28, 2017
@Gargron Gargron deleted the feature-flexible-status-length-count branch July 28, 2017 22:06
This was referenced Aug 2, 2017
Closed
Closed
Closed
@bjtitus bjtitus mentioned this pull request Oct 23, 2017
Merged
@clarfonthey clarfonthey mentioned this pull request Jan 5, 2018
Closed
2 tasks
@digitalcircuit digitalcircuit mentioned this pull request Mar 23, 2022
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nightpool nightpool nightpool left review comments

@progval progval progval left review comments

@kensanata kensanata kensanata left review comments

@krainboltgreene krainboltgreene krainboltgreene approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Gargron @nightpool @krainboltgreene @progval @kensanata