feat: introduce navigator.language #12322

AnInternetTroll · 2021-10-04T20:59:57Z

Link to the spec: https://html.spec.whatwg.org/multipage/system-state.html#dom-navigator-language-dev
~~To access navigator.language you must pass --allow-env as it can lead to fingerprinting.~~

What this PR doesn't add is languagechange which quite frankly is outside of my skill level.

CLAassistant · 2021-10-04T21:00:02Z

All committers have signed the CLA.

AnInternetTroll · 2021-10-04T21:02:18Z

The spec says the following

If the user is using an anonymizing service, then the value "en-US" is suggested; if all users of the service use that same value, that reduces the possibility of distinguishing the users from each other.

And MDN says:

Returns a DOMString representing the preferred language of the user, usually the language of the browser UI. The null value is returned when this is unknown.

I would argue that if --allow-env isn't passed then navigator.language should be en-US. or null instead of throwing an error.

Also an additional note, on unix this would be more or less the same as getting the LANG env variable, so maybe --allow-env=LANG should allow navigator.language to return the correct language. This should be discussed though as it is not how it works on windows.

crowlKats

Missing WorkerNavigator typings and property definition in 99_main

runtime/js/99_main.js

…`WorkerNavigator`

lucacasonato · 2021-10-05T21:13:17Z

Actually, this can be enabled by default (no permissions), because the default language can already be extracted via Intl global.

bartlomieju · 2021-10-08T10:09:33Z

runtime/ops/os.rs

+fn op_languages(
+  _state: &mut OpState,
+  _: (),
+  _: (),
+) -> Result<Vec<String>, AnyError> {


If locale_config caches the result it means that this value cannot change after the first call. I'd suggest to remove this op and forward the value to JS in BootstrapOptions struct.

CC @lucacasonato

lucacasonato

Could you also add a test for navigator.languages?

lucacasonato

~~Looks good now. Thanks Ben.~~

@AnInternetTroll

Sorry @AnInternetTroll, accidentally left my review comment on the wrong PR.

crowlKats

languages should be frozen

stale · 2021-12-27T20:38:11Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale · 2022-02-26T20:28:36Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

AnInternetTroll · 2022-04-29T05:36:44Z

Not stale, work has been done in rusty_v8 denoland/rusty_v8#953

stale · 2022-07-01T12:32:36Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale · 2022-09-20T15:46:25Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

bartlomieju · 2022-10-08T15:22:42Z

This is now done and should be ready to land. @lucacasonato @crowlKats please review

lucacasonato

LGTM

bartlomieju

LGTM

addressed

zbraniecki

fly-by review.

zbraniecki · 2022-10-27T21:59:14Z

runtime/worker_bootstrap.rs

@@ -47,6 +48,7 @@ impl Default for BootstrapOptions {
      enable_testing_features: Default::default(),
      debug_flag: Default::default(),
      ts_version: Default::default(),
+      locale: "en-EN".to_string(),


this seems like an error - there's no region EN.

Thanks! Silly mistake on my part, I'll fix it

@zbraniecki

Pointed by @zbraniecki in #12322 (comment), I made a mistake with default locale value.

AnInternetTroll added 2 commits October 4, 2021 22:52

Add navigator.language and tests

72dcc05

Merge github.com:AnInternetTroll/deno

c114fde

crowlKats requested changes Oct 4, 2021

View reviewed changes

runtime/js/99_main.js Outdated Show resolved Hide resolved

runtime/js/99_main.js Outdated Show resolved Hide resolved

kitsonk requested a review from lucacasonato October 4, 2021 21:04

AnInternetTroll added 3 commits October 4, 2021 23:06

Add typings and a fancier function thx @crowlKats

3955a42

Add configurable and enumerable properties and add language to …

4576676

…`WorkerNavigator`

Re-format code

9e35957

Remove --allow-env requirement for navigator.language

fcdbd92

bartlomieju reviewed Oct 8, 2021

View reviewed changes

AnInternetTroll added 2 commits October 10, 2021 14:22

Merge branch 'main' of github.com:denoland/deno

cde6093

Move from making an op call to having a Bootstrap option

e1acc7f

AnInternetTroll force-pushed the main branch from 26db845 to e1acc7f Compare October 10, 2021 14:49

Merge branch 'main' of github.com:denoland/deno

792e42d

lucacasonato reviewed Oct 11, 2021

View reviewed changes

lucacasonato previously approved these changes Oct 11, 2021

View reviewed changes

crowlKats previously requested changes Oct 11, 2021

View reviewed changes

AnInternetTroll added 5 commits October 11, 2021 19:45

Add test for navigator.languages and freeze it

c63e28b

Remove debug print

3408957

Set LANGUAGE env variable in tests

833e51d

Fix test to use LC_ALL instead of LANG

05a8829

Merge github.com:denoland/deno

0154976

stale bot added the stale label Dec 27, 2021

bartlomieju removed the stale label Dec 28, 2021

stale bot added the stale label Feb 26, 2022

stale bot removed the stale label Apr 29, 2022

stale bot added the stale label Jul 1, 2022

bartlomieju removed the stale label Jul 1, 2022

stale bot added the stale label Sep 20, 2022

bartlomieju added 2 commits September 27, 2022 00:55

Merge branch 'main' into AnInternetTroll/main

e244948

update to latest rusty_v8

f33c85a

stale bot removed the stale label Sep 30, 2022

cleanup

81f4707

bartlomieju mentioned this pull request Sep 30, 2022

feat: Add v8::icu::get_language_tag() denoland/rusty_v8#953

Merged

bartlomieju added this to the 1.27 milestone Oct 5, 2022

Merge branch 'main' into AnInternetTroll/main

683020c

bartlomieju requested review from crowlKats and lucacasonato October 8, 2022 15:25

lucacasonato approved these changes Oct 8, 2022

View reviewed changes

bartlomieju and others added 6 commits October 9, 2022 10:27

try to fix test

7eb1523

Use LC_ALL in test instead of LANG and LANGUAGE

10d548e

Format

6d990ae

Merge branch 'main' into AnInternetTroll/main

bd8ee25

fix lint

c98844a

Merge branch 'main' into main

9cec295

bartlomieju approved these changes Oct 18, 2022

View reviewed changes

bartlomieju merged commit 1a0c7ed into denoland:main Oct 18, 2022

zbraniecki reviewed Oct 27, 2022

View reviewed changes

bartlomieju mentioned this pull request Oct 28, 2022

fix: change default locale value #16463

Merged

bartlomieju added a commit that referenced this pull request Oct 28, 2022

fix: change default locale value (#16463)

065f9cc

Pointed by @zbraniecki in #12322 (comment), I made a mistake with default locale value.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: introduce navigator.language #12322

feat: introduce navigator.language #12322

AnInternetTroll commented Oct 4, 2021 •

edited

Loading

CLAassistant commented Oct 4, 2021 •

edited

Loading

AnInternetTroll commented Oct 4, 2021 •

edited

Loading

crowlKats left a comment

lucacasonato commented Oct 5, 2021

bartlomieju Oct 8, 2021

lucacasonato left a comment

lucacasonato left a comment •

edited

Loading

crowlKats left a comment

stale bot commented Dec 27, 2021

stale bot commented Feb 26, 2022

AnInternetTroll commented Apr 29, 2022

stale bot commented Jul 1, 2022

stale bot commented Sep 20, 2022

bartlomieju commented Oct 8, 2022

lucacasonato left a comment

bartlomieju left a comment

zbraniecki left a comment

zbraniecki Oct 27, 2022

bartlomieju Oct 28, 2022

feat: introduce navigator.language #12322

feat: introduce navigator.language #12322

Conversation

AnInternetTroll commented Oct 4, 2021 • edited Loading

CLAassistant commented Oct 4, 2021 • edited Loading

AnInternetTroll commented Oct 4, 2021 • edited Loading

crowlKats left a comment

Choose a reason for hiding this comment

lucacasonato commented Oct 5, 2021

bartlomieju Oct 8, 2021

Choose a reason for hiding this comment

lucacasonato left a comment

Choose a reason for hiding this comment

lucacasonato left a comment • edited Loading

Choose a reason for hiding this comment

crowlKats left a comment

Choose a reason for hiding this comment

stale bot commented Dec 27, 2021

stale bot commented Feb 26, 2022

AnInternetTroll commented Apr 29, 2022

stale bot commented Jul 1, 2022

stale bot commented Sep 20, 2022

bartlomieju commented Oct 8, 2022

lucacasonato left a comment

Choose a reason for hiding this comment

bartlomieju left a comment

Choose a reason for hiding this comment

zbraniecki left a comment

Choose a reason for hiding this comment

zbraniecki Oct 27, 2022

Choose a reason for hiding this comment

bartlomieju Oct 28, 2022

Choose a reason for hiding this comment

AnInternetTroll commented Oct 4, 2021 •

edited

Loading

CLAassistant commented Oct 4, 2021 •

edited

Loading

AnInternetTroll commented Oct 4, 2021 •

edited

Loading

lucacasonato left a comment •

edited

Loading