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

Supporting custom cultures that are not pre-defined in options.SupportedCultures #2649

Closed
aspnet-hello opened this issue Jan 2, 2018 · 16 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one
Milestone

Comments

@aspnet-hello
Copy link

From @chaim770 on Wednesday, April 13, 2016 2:18:44 AM

Following this StackOverflow question:
http://stackoverflow.com/questions/36590969/enabling-client-based-culture-in-asp-net-core

It's currently impossible to let the CultureInfo be determined automatically from what the client specify in the Accept-Language header (as it used to be using <system.web.globalization enableClientBasedCulture> in Web.config).

The way it is now, I have to pre-register all the cultures I'm planning to support, which renders AcceptLanguageHeaderRequestCultureProvider quite useless.

The following is the culprit:
https://github.com/aspnet/Localization/blob/b52e33caa7927093dc3ec540fd3282f4f69ae169/src/Microsoft.Extensions.Globalization.CultureInfoCache/CultureInfoCache.cs#L34

Copied from original issue: aspnet/Localization#237

@aspnet-hello aspnet-hello added this to the Backlog milestone Jan 2, 2018
@aspnet-hello aspnet-hello added enhancement This issue represents an ask for new feature or an enhancement to an existing one repo:Localization labels Jan 2, 2018
@aspnet-hello
Copy link
Author

From @hishamco on Wednesday, April 13, 2016 6:37:50 AM

I think after design change that discussed here #111 we should to register the supported culture instead of looking into Culture Info list which is removed after this change

@aspnet-hello
Copy link
Author

From @omuleanu on Wednesday, April 13, 2016 7:26:50 AM

@hishamco this change is already done, it's in rc1-final, the version that we're having trouble with

@aspnet-hello
Copy link
Author

From @hishamco on Wednesday, April 13, 2016 7:53:54 AM

I know @omuleanu, but while web.config is no longer used, is there any reason to have such enableClientBasedCulture?!!

@aspnet-hello
Copy link
Author

From @omuleanu on Wednesday, April 13, 2016 8:07:24 AM

@hishamco yes, a lot of people need that, basically everyone that was/is using culture=auto in web.config and want the same behavior after porting their app to the new asp.net

I need it, to automatically get the user's ShortDatePattern, ShortTimePattern, DateTimeFormat, NumberDecimalSeparator, TimeSeparator

@aspnet-hello
Copy link
Author

From @hishamco on Wednesday, April 13, 2016 8:36:21 AM

There 're a lot of changes in ASP.NET Core, and there's no guarantee that every feature before will be the same in the new world, nothing but what if you create a culture info list from Accept-Language header and pass it to RequestLocalizationOptions this way it collect the cultures automatically instead of hard coded.
I'm not sure if there's a change in the future to support this out of the box, @DamianEdwards any idea sir?

@aspnet-hello
Copy link
Author

From @Eilon on Wednesday, April 13, 2016 9:51:17 AM

This is currently by design in ASP.NET Core for the reasons mentioned in aspnet/Localization#111 (comment).

If we do this, it would most likely be a different middleware, or perhaps an option on the current middleware, but it would have negative performance implications.

@aspnet-hello
Copy link
Author

From @chaim770 on Wednesday, April 13, 2016 10:08:37 AM

@Eilon

it would have negative performance implications.

Why? Can you please elaborate?

@aspnet-hello
Copy link
Author

From @Eilon on Wednesday, April 13, 2016 10:17:43 AM

@chaim770 I go into more detail in aspnet/Localization#111 (comment). The key reason is that ASP.NET has to cache every culture, so it's a trade-off between not caching and having slow performance, or caching but having the cache grow infinitely. Neither is a good option, so we chose to cache, but to limit the size of the cache by requiring apps to specify which items can be cached.

@aspnet-hello
Copy link
Author

From @hishamco on Tuesday, October 25, 2016 7:58:03 PM

@elion IMHO if the Accept-Language sent with tens of cultures, typically you’re only interested in the first one for setting the language as that’s the primary language. In this case we avoid the performance impact. If it's need I can write a simple middlware in my repo or in the entropy repo to prove the point

@aspnet-hello
Copy link
Author

From @DamianEdwards on Tuesday, October 25, 2016 8:19:01 PM

That's not the point. The issue was a single client sending lots of requests (000's) for different cultures. Creating culture objects isn't cheap so we avoid doing it unless we need to. Culture works differently on .NET Core than it did in .NET Framework on Windows, where the list was fixed based on what the OS supported. That said, the logic in this middleware is pretty trivial, and if an app wants to have the format culture set to whatever the client asks for it's very easy for the app to add that custom logic in a simple middleware.

@aspnet-hello
Copy link
Author

From @hishamco on Tuesday, October 25, 2016 8:30:22 PM

I agree with you that creating a culture object isn't cheap, but what I suggest is a workaround to migrate his old ASP.NET Application to ASP.NET Core 😄

@aspnet-hello
Copy link
Author

From @hikalkan on Thursday, December 8, 2016 11:33:19 PM

In our multitenant application, a tenant can add a new language from UI and edit localization texts for it. But the tenant can not switch to the new language unless we start IIS (we definetely don't want that). I think this is a very common case and there should be a built-in solution.
Thanks.

@aspnet-hello
Copy link
Author

From @hishamco on Saturday, March 4, 2017 11:09:28 AM

As I can see here https://github.com/aspnet/Localization/blob/dev/src/Microsoft.AspNetCore.Localization/AcceptLanguageHeaderRequestCultureProvider.cs#L42-L47, we are interest to the top MaximumAcceptLanguageHeaderValuesToTry even the client has tens of cultures, so why this has a performance impact?!!

@aspnet-hello
Copy link
Author

From @Eilon on Saturday, March 4, 2017 4:14:21 PM

@hishamco generally speaking, accepting unbounded inputs is a bad practice, whether for perf or security.

@aspnet-hello
Copy link
Author

From @hishamco on Saturday, March 4, 2017 11:42:40 PM

Either ways the AcceptLanguageHeaderRequestCultureProvider accepts the user cultures that set via Accept-Language header, whether they are correct or not, so if AcceptLanguageHeaderRequestCultureProvider is the winning provider, after that we set the current culture if its correct or fallback to the default culture.

What I mean in my last comment is we can consider the provided culture if it's correct while we care about the top 3 which is the default value

Regardless of what I wrote before while the aim of this issue is Supporting custom cultures that are not pre-defined in options.SupportedCultures, so we can go further by allowing the devs and applications authors to provide a custom cultures via configuration file, using Configuration & Options APIs, like what we have seen in EF, in this case we reduce (or eliminate) the impact because these cultures values will provided from the application itself not the end users themselves

@mkArtakMSFT
Copy link
Member

Thanks for contacting us. We're closing this issue as there was no community involvement here for quite a while now.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests

4 participants