adding draft for locale api review #2925

srwei · 2022-11-01T20:53:56Z

No description provided.

srwei · 2022-11-03T18:10:16Z

@david-risney
I'm thinking I should add an example or a reference to the Windows API that gets the OS region if they want to always use whatever is set in the OS. Would this be a good place to put it? or should I put this in the documentation in Microsoft.Web.WebView2.Core/documentation.md?

david-risney · 2022-11-03T18:30:39Z

Yes. I would recommend updating the two samples in this doc (& in the sample apps)

srwei · 2022-11-03T21:25:21Z

Thanks. Updated here and in the .idl file.

david-risney · 2022-11-10T23:31:30Z

specs/LocaleRegion.md

+* The intended value for `LocaleRegion` is in the format of `language[-country]` where `language` is the 
+2-letter code from [ISO 639](https://www.iso.org/iso-639-language-codes.html) and `country` is the 
+2-letter code from [ISO 3166](https://www.iso.org/standard/72482.html).
+* By default, the `LocaleRegion` will be set as the value for the Limited option in the browser.


Probably not helpful to refer to the 'Limited option in the browser' since that won't be a known thing. We can just describe this on its own terms.

david-risney · 2022-11-10T23:32:51Z

specs/LocaleRegion.md

+2-letter code from [ISO 639](https://www.iso.org/iso-639-language-codes.html) and `country` is the 
+2-letter code from [ISO 3166](https://www.iso.org/standard/72482.html).
+* By default, the `LocaleRegion` will be set as the value for the Limited option in the browser.
+That means that if the OS region and the display language share the same language code, 


This info about the default value needs to be included in the ref docs below (the /// comments above the IDL method). Info in the ref docs doesn't need to be duplicated in the Description.

david-risney · 2022-11-10T23:36:46Z

specs/LocaleRegion.md

+2-letter code from [ISO 3166](https://www.iso.org/standard/72482.html).
+* By default, the `LocaleRegion` will be set as the value for the Limited option in the browser.
+That means that if the OS region and the display language share the same language code, 
+like 'en-GB' and 'en-US', the value will be set to the OS region.


It would be good to have a table of different options and the result. Something like the following (but with more rows) in the /// ref doc comments

OS Region WebView2 Language Default WebView2 LocaleRegion

en-GB en-US en-GB

... ... ...

Got it. Will add this as part of the experimental PR

david-risney · 2022-11-10T23:37:56Z

specs/LocaleRegion.md

+  /// The Windows API `GetLocaleInfoEx` can be used if the LocaleRegion value
+  /// is always set to match the OS region
+  ///
+  /// ```cpp


Instead of embedding the code directly, we generally use snippets syntax (see other APIs as examples) that will pull the snippet code out of the sample apps.

Was talking to @peiche-jessica about this, if the intention is not to include some particular code in the sample app but to have it for the reference doc (like usage of GetLocaleInfoEx), then shouldn't we add it here as a comment and then it will appear as code in the reference doc only?

Its better to have it at least in the reference documentation, but generally we want to copy the sample code out of the sample app so end devs can easily try out the code or search and find the code on github. Why don't we want to include this sample code in the sample app?

Also it would be nice to complete these samples to show how you would actually use them with the property.

david-risney · 2022-11-10T23:39:19Z

specs/LocaleRegion.md

+interface ICoreWebView2StagingControllerOptions : IUnknown {
+  /// Interface for locale region that is updated through the ControllerOptions
+  /// API.
+  /// The default region for WebView.  It applies to browser UI such as


Does it actually apply to browser UI? I thought it was only specific JavaScript APIs? Speaking of which, you should explicitly list the exact JavaScript APIs that are affected.

Yeah I might have been too general here. It affects the Intl.DateTimeFormat() Locale object which gets reflected in the UI for time/dates/other string formatting if it's used.

david-risney · 2022-11-10T23:41:41Z

specs/LocaleRegion.md

+  /// If V8 cannot find any matching locale on the input value, it will default
+  /// to the display language as the locale.
+  ///
+  /// The Windows API `GetLocaleInfoEx` can be used if the LocaleRegion value


I would rephrase to make this more OS independent like "Use OS specific APIs to determine the OS region to use with this property if you want to match the OS. For example in Windows you can use the GetLocaleInfoEx method."

Also, can we recommend the methods to use for .NET and WinRT? .NET is likely our most popular language.

david-risney · 2022-11-10T23:42:06Z

specs/LocaleRegion.md

+  // MSOWNERS: [email protected]
+  [propget] HRESULT LocaleRegion([out, retval] LPWSTR* locale);
+
+  /// Sets the `Region` property.


Suggested change

/// Sets the `Region` property.

/// Sets the `LocaleRegion` property.

david-risney · 2022-11-10T23:43:17Z

specs/LocaleRegion.md

+```cpp
+[uuid(0c9a374f-20c3-4e3c-a640-67b78a7e0a48), object, pointer_default(unique)]
+interface ICoreWebView2StagingControllerOptions : IUnknown {
+  /// Interface for locale region that is updated through the ControllerOptions


This first sentence isn't really necessary - should be sort of implied by the ref doc which will include links, the name of the interface and so on.

david-risney · 2022-11-10T23:43:50Z

specs/LocaleRegion.md

+interface ICoreWebView2StagingControllerOptions : IUnknown {
+  /// Interface for locale region that is updated through the ControllerOptions
+  /// API.
+  /// The default region for WebView.  It applies to browser UI such as


Suggested change

/// The default region for WebView. It applies to browser UI such as

/// The default region for the WebView2. It applies to browser UI such as

specs/LocaleRegion.md

david-risney · 2022-11-17T21:47:09Z

specs/LocaleRegion.md

+  /// The Windows API `GetLocaleInfoEx` can be used if the LocaleRegion value
+  /// is always set to match the OS region
+  ///
+  /// ```cpp


Its better to have it at least in the reference documentation, but generally we want to copy the sample code out of the sample app so end devs can easily try out the code or search and find the code on github. Why don't we want to include this sample code in the sample app?

david-risney · 2022-11-17T21:51:02Z

specs/LocaleRegion.md

+  /// so changes will not be reflected in the existing webviews. They will need to closed
+  /// and reopened in order to see the changes reflected from using the new creation environment.
+  ///
+  /// Validation is done on the V8 engine to match on the closest locale


Saying that the V8 engine is doing this isn't helpful unless how the V8 engine does it is well known or well documented. Otherwise you can just talk in terms of this API and how validation of the parameter is performed.

I spoke to the owner of the browser side and they said they think the validation processing is done by ICU. And linked me this to reference some documentation here: https://source.chromium.org/chromium/chromium/src/+/main:third_party/icu/source/common/unicode/locid.h
Is it okay to not mention the specific examples and just link to this? looking through the link, there's no concrete explanation of the matchign logic

david-risney · 2022-11-17T21:54:37Z

specs/LocaleRegion.md

+  /// and reopened in order to see the changes reflected from using the new creation environment.
+  ///
+  /// Validation is done on the V8 engine to match on the closest locale
+  /// from the passed in locale region value. For example, passing in "en_gb"


This paragraph does not make it clear what is actually allowed or what validation is actually performed. It sounds like '_' is allowed instead of '-'? Are there many other things like that? Or is the _ syntax a different form language/country code format that we also accept and need to document? Is this the same as the syntax and validation performed for the Language property?

david-risney · 2022-11-17T21:58:14Z

specs/LocaleRegion.md

+  /// 639](https://www.iso.org/iso-639-language-codes.html) and `country` is the
+  /// 2-letter code from [ISO 3166](https://www.iso.org/standard/72482.html).
+  ///
+  /// This property will update the environment creation. This is global and immutable, 


nit: Just wording suggestions

Suggested change

/// This property will update the environment creation. This is global and immutable,

/// This property sets the region for a CoreWebView2Environment during its creation.

david-risney · 2022-11-17T22:00:42Z

specs/LocaleRegion.md

+  /// 2-letter code from [ISO 3166](https://www.iso.org/standard/72482.html).
+  ///
+  /// This property will update the environment creation. This is global and immutable, 
+  /// so changes will not be reflected in the existing webviews. They will need to closed


Instead of global let's say that it applies to the environment and cannot be changed for that environment. (An app could have more than one environment so the property isn't global in that sense)

Suggested change

/// so changes will not be reflected in the existing webviews. They will need to closed

/// Creating a new CoreWebView2Environment object that connects to an already running browser process cannot

/// change the region previously set by an earlier CoreWebView2Environment. The CoreWebView2Environment

/// and all associated webview2 objects will need to closed

specs/LocaleRegion.md

david-risney · 2022-11-17T22:01:25Z

specs/LocaleRegion.md

+  /// The Windows API `GetLocaleInfoEx` can be used if the LocaleRegion value
+  /// is always set to match the OS region
+  ///
+  /// ```cpp


Also it would be nice to complete these samples to show how you would actually use them with the property.

david-risney · 2022-11-17T22:02:31Z

specs/LocaleRegion.md

+2-letter code from [ISO 3166](https://www.iso.org/standard/72482.html).
+
+# Examples
+```cpp


The two examples in C++ & C# should do the same thing. Here the C++ just sets up the controller options, and the C# one also calls create async, and setapplocale

srwei · 2022-11-22T20:00:07Z

@david-risney created a PR to include an option to use OSRegion for the LocaleRegion value in the SampleApp. Because it's now part of the code snippet, do I still need to include the examples in the comments for c++ and c#?
PR link: https://microsoft.visualstudio.com/Edge/_git/chromium.src/pullrequest/8154109

adding draft for locale api review

3d650ce

srwei requested a review from david-risney November 1, 2022 20:53

david-risney added the API Proposal Review WebView2 API Proposal for review. label Nov 1, 2022

adding the updated .idl documentation

e8814c1

david-risney requested changes Nov 10, 2022

View reviewed changes

addressed comments

cdf93ac

srwei requested a review from david-risney November 14, 2022 21:07

update idl comments

ac7843b

david-risney requested changes Nov 17, 2022

View reviewed changes

addressing comments

bb757b3

srwei requested a review from david-risney November 29, 2022 18:41

removing validation comments and adding - and _ to expected input

3aec1fe

srwei merged commit a128202 into api-locale-region Dec 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding draft for locale api review #2925

adding draft for locale api review #2925

srwei commented Nov 1, 2022

srwei commented Nov 3, 2022

david-risney commented Nov 3, 2022

srwei commented Nov 3, 2022

david-risney Nov 10, 2022

david-risney Nov 10, 2022

david-risney Nov 10, 2022

srwei Nov 11, 2022

david-risney Nov 10, 2022

srwei Nov 11, 2022

david-risney Nov 17, 2022

david-risney Nov 17, 2022

david-risney Nov 10, 2022

srwei Nov 11, 2022

david-risney Nov 10, 2022

david-risney Nov 10, 2022

david-risney Nov 10, 2022

david-risney Nov 10, 2022

david-risney Nov 17, 2022

david-risney Nov 17, 2022

srwei Nov 22, 2022

david-risney Nov 17, 2022

david-risney Nov 17, 2022

david-risney Nov 17, 2022

david-risney Nov 17, 2022

david-risney Nov 17, 2022

srwei commented Nov 22, 2022 •

edited

Loading

	/// Sets the `Region` property.
	/// Sets the `LocaleRegion` property.

	/// The default region for WebView. It applies to browser UI such as
	/// The default region for the WebView2. It applies to browser UI such as

	/// This property will update the environment creation. This is global and immutable,
	/// This property sets the region for a CoreWebView2Environment during its creation.

-  /// so changes will not be reflected in the existing webviews. They will need to closed
+  /// Creating a new CoreWebView2Environment object that connects to an already running browser process cannot
+  /// change the region previously set by an earlier CoreWebView2Environment. The CoreWebView2Environment
+  /// and all associated webview2 objects will need to closed

adding draft for locale api review #2925

adding draft for locale api review #2925

Conversation

srwei commented Nov 1, 2022

srwei commented Nov 3, 2022

david-risney commented Nov 3, 2022

srwei commented Nov 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srwei commented Nov 22, 2022 • edited Loading

srwei commented Nov 22, 2022 •

edited

Loading