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

Customize scheme 1 #278

Closed
jeancroy opened this issue Jun 30, 2016 · 19 comments · Fixed by #285
Closed

Customize scheme 1 #278

jeancroy opened this issue Jun 30, 2016 · 19 comments · Fixed by #285

Comments

@jeancroy
Copy link
Contributor

jeancroy commented Jun 30, 2016

Basically I want to get rid of this step

4. The request contains an Accept-Language header with a language
    that matches (exactly or loosely) one of the application languages.

The context is "fr-ca" : a bunch of french speaking people in a sea of English speaking territory.
Official language of business is french.
A lot of computer system are in English. Like 50/50.
Among other things choosing English software help to search on web for settings/feature XYZ.

Basically the language of the os/browser is disconnected from user intent. Most of our target user don't have any idea about advanced settings to adjust asked language.


Then I realize Scheme1 without step4 is almost Scheme2 + cookie.
So I tried to implement cookies in DetermineDefaultLanguageFromRequest delegate.
Not complex but also a failure. This is because whatever the ouput of this is hidden as being the default.
I tough this was a bug until I see it documented:

The default URL Localization scheme (Scheme1) will show the language tag in the URL always; 
an alternative scheme, Scheme2, will show the language tag only if it is not the default.

So basically the feedback is stuff is glued together.
Url presentation is tied to language detection source.
Cookie language detection is tied to request language detection.

So it would be nice to have separate Boolean for behavior.

DetectLanguageFromCookie
DetectLanguageFromHeader
HideDefaultLanguageInUrl

Sheme1 could map as (true,true,false)
Scheme2 map as (false,false,true)

And after setting the scheme we could refine the behavior

@turquoiseowl
Copy link
Owner

Yes, I'm with your analysis 100%. It should have been developed like that in the first place TBH.

I'm a bit tied up with other stuff at the moment. Are you able to have a crack at it yourself?

@jeancroy
Copy link
Contributor Author

I can give it a try. I do have one question.

Here it show GetRequestUserLanguages should choose between the request and the PAL

public static LanguageTag GetInferredLanguage(this System.Web.HttpContextBase context)
{
// langtag = best match between
// 1. Inferred user languages (cookie and Accept-Language header)
// 2. App Languages.
LanguageTag lt = null;
System.Web.HttpCookie cookie_langtag = context.Request.Cookies.Get("i18n.langtag");
if (cookie_langtag != null) {
lt = LanguageHelpers.GetMatchingAppLanguage(cookie_langtag.Value); }
if (lt == null) {
lt = LanguageHelpers.GetMatchingAppLanguage(context.GetRequestUserLanguages()); }
if (lt == null) {
throw new InvalidOperationException("Expected GetRequestUserLanguages to fall back to default language."); }
return lt;
}

But in the body there's this comment

// NB: originally we passed LocalizedApplication.Current.DefaultLanguageTag
// here as the second parameter i.e. to specify the PAL. However, this was
// found to be incorrect when operating i18n with EarlyUrlLocalization disabled,
// as SetPrincipalAppLanguageForRequest was not being called, that normally
// overwriting the PAL set erroneously here.

So i'm not sure how LocalizedApplication.Current.DefaultLanguageTag ever surface as the default choice.

@jeancroy
Copy link
Contributor Author

Also would need some guidance on life cycle of those behavior boolean.

Some options I can think of:

  1. Real Boolean. Resolve early. For exam listen to change of scheme.
  2. Nullable. Resolve Late. behavior = manualOption ?? getDefautlForScheme(currentScheme) .
  3. Don't bother about behavior boolean unless scheme is scheme.Custom.

Option 3 feel a bit like a patch, giving up the idea of base scheme + refine.
But it help ensure we don't introduce subtle behavior change in existing installation.

@turquoiseowl
Copy link
Owner

There are two sets of language tags: Application Languages and User Languages. The former relates to those supported by your webapp (what's available); the latter are those requested by the browser/user-agent (what's wanted).

The LocalizedApplication.Current.DefaultLanguageTag value is relevant in terms of ApplicationLanguages and is added to that set in addition to any locales/translations you add to your app. It isn't added to User Languages.

The job of i18n is then to make a best match between those two sets of languages.

@turquoiseowl
Copy link
Owner

WRT booleans, how about somehting like:

    [Flags]
    public enum UrlLocalizationScheme
    {
        _DetectLanguageFromCookieFlag = 0x01,
        _DetectLanguageFromHeaderFlag = 0x02,
        _HideDefaultLanguageInUrlFlag = 0x04,

        Void = 0,
        Scheme1 = _DetectLanguageFromCookieFlag | _DetectLanguageFromHeaderFlag,
        Scheme2 = _HideDefaultLanguageInUrlFlag,
        Scheme3 = 0,
    }

@turquoiseowl
Copy link
Owner

Thinking about this a bit more, are you sure Scheme2 excludes Accept-Language languages from consideration? IIRC the only thing it modifies is whether or not the langtag is shown in the URL when the default app language was current. That was its original purpose anyhow.

@turquoiseowl
Copy link
Owner

And some more: the UrlLocalization and TextLocalization are somewhat distinct parts of i18n. Perhaps the DetectLanguageFromHeaderFlag settings etc. are not strictly related to UrlLocalization and can best be implemented as a simple booleans in the app settings or static flag properties on the LocalizedApplication class.

@jeancroy
Copy link
Contributor Author

jeancroy commented Jun 30, 2016

Thinking about this a bit more, are you sure Scheme2 excludes Accept-Language languages from consideration?

My default browser send:
Accept-Language:en-US,en;q=0.8

The default language is
i18n.LocalizedApplication.Current.DefaultLanguage = "fr";

Using Scheme 2 french is applied,
Using Scheme1 english is applied.

So current behavior is scheme2 exclude Accept-Language.


As far as design perspective, I'm not sure it should be linked either. Scheme1/2 could control only display, _DetectLanguageFromCookieFlag , _DetectLanguageFromRequestFlag could be set independently

Then this package have tons of download on nugget so I'm not sure it's that good to change default

@turquoiseowl
Copy link
Owner

Yes, you're right: a side effect of Scheme2 is that a langtag will be derived from the URL, whether or not the URL contains a langtag. Therefore, the cookie and Accept-Language header never get a look in.

The simplest thing looks like two corresponding flags in the HttpContextExtensions.GetInferredLanguage method.

@turquoiseowl
Copy link
Owner

HttpContextExtensions.GetInferredLanguage then needs to fallback explicitly on LocalizedApplication.Current.DefaultLanguageTag if/when UserLanguages (i.e. headers) are disabled.

@Jandev
Copy link
Contributor

Jandev commented Aug 1, 2016

It appears I'm stumbling across the mentioned issue also.
We also want/need to control if the Headers will be parsed for a language or not.

Can you enlighten us if there has been any progress in the development? If not I'll just have to create it myself :-)

@turquoiseowl
Copy link
Owner

I defer to @jeancroy on this.

@jeancroy
Copy link
Contributor Author

jeancroy commented Aug 1, 2016

It kinda depend how universal we want to fix this.
Do we agree on a minimum fix that only skip the header ?

@turquoiseowl
Copy link
Owner

Do we agree on a minimum fix that only skip the header ?

Yes. Single-purpose flag on LocalizedApplication maybe.

@Jandev
Copy link
Contributor

Jandev commented Aug 2, 2016

Sounds good! I'll discuss internally if we can wait for this feature or need to develop something similar in the meantime.

@Jandev
Copy link
Contributor

Jandev commented Aug 2, 2016

I've forked the project and made a small change concerning the Accept-Language header by creating a delegate which a developer can override if necessary. Similar to the UrlLocalizer code.
Link: LasaulecBV@4d4e639

It's not based on the flags, like discussed over here. For now our team can work with this change I made. If you want I can create a PR (with comments and stuff added of course), but perhaps the flags might be a bit easier (for now).

@turquoiseowl
Copy link
Owner

Even better than a flag! I would be very pleased to receive a PR.

If you've time, a note on this feature in the README would be cool.

@Jandev
Copy link
Contributor

Jandev commented Aug 3, 2016

Sure, no problem. I'll get to it tomorrow or the day after.
In my current branch I've also upgraded the project to VS2015. Is this a problem?
Otherwise I can make the changes by not converting it to VS2015 if necessary.

@turquoiseowl
Copy link
Owner

Yes, if we can keep the project open to VS2013 users for now that would be great. Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants