International Domain Names (IDN) Support - #2034
Conversation
BundleMonUnchanged files (7)
No change in files bundle size Final result: ✅ View report in BundleMon website ➡️ |
| |> cast(attrs, [:domain, :timezone]) | ||
| |> validate_required([:domain, :timezone]) | ||
| |> validate_format(:domain, ~r/^[a-zA-Z0-9\-\.\/\:]*$/, | ||
| |> validate_format(:domain, ~r/^[-.\\:\p{L}0-9]*$/u, |
There was a problem hiding this comment.
I replaced a-zA-Z with p{L} which stands for Unicode letter (in any alphabet).
|
@vinibrsl nice! for marketing/communication purposes: when this is merged, we will support any character for the domain name? or this adds support for some specific international characters only? basically need to know what to communicate to people |
@metmarkosaric this pull request adds support for any Unicode character in the "letter" category, not any character. In other words, a letter in any language. Plus numbers, dots, and slashes. |
ukutaht
left a comment
There was a problem hiding this comment.
The PR looks good overall!
The new encoding function is used in a couple places but there are many many more places where we encode domains. Try running git grep 'URI.encode_www_form' in the project to see all of them.
I need some convincing in the first place that the old function isn't going to work:
iex(7)> URI.encode_www_form("wèbsite.com/wat")
"w%C3%A8bsite.com%2Fwat"When I paste this into the browser, I get:
Which looks correct to me. So maybe we don't need to re-do the encoding?
|
I've always thought the techincal debt we have is not using the Phoenix Route helpers which automatically url-encode your params. So instead of: |> redirect(to: "/#{URI.encode_www_form(site.domain)}/settings/danger-zone")We could do |> redirect(to: Routes.site_path(conn, :settings_danger_zone, site.domain))But I think the behaviour between them is exactly the same |
You're right, we don't need the new encoding function for the URLs to work. However, there are two places where the escaped URL is displayed: (1) dashboard visibility toggle and (2) shared links. Here's how they look without the new function:
At least for the visibility toggle we should have some kind of escaping. I moved the new function to be used only in that view (d49fb6a). Based on your suggestion I also changed Phoenix links and buttons to the generated
|
Changes
This PR adds support for IDNs as discussed in #993. I replaced the latin alphanumeric validation with a more broad one, supporting other alphabets. I also made some changes to URI encoding to make sure our URLs are still good looking.
Tests
Changelog
Documentation
Dark mode