1486258 - Regression tests to check that new Intl APIs respect privacy.spoof_english

Status: RESOLVED FIXED

(Core :: JavaScript: Internationalization API, defect, P2)

Description

[Arthur Edelstein [:arthur]]

When "privacy.spoof_english" is set to 2, it is expected that Intl APIs hide the user's locale and report values consistent with the "en-US" locale instead. I'd like to propose adding some regression tests for new Intl APIs that were introduced after the "privacy.spoof_english" mechanism was added. The APIs are:
* hourCycle
* PluralRules
* NumberFormat.formatToParts

(These APIs already respect "privacy.spoofEnglish", so no changes to the implementation is needed.)

Comment 1

[Arthur Edelstein [:arthur]]

Here's the Tor ticket where manual tests were performed: https://trac.torproject.org/26611

Comment 2

[Thorin [:thorin]]

(In reply to Arthur Edelstein [:arthur] from comment #1)

Here's the Tor ticket where manual tests were performed:
https://trac.torproject.org/26611

Here's all the manual tests and more: https://ghacksuserjs.github.io/TorZillaPrint/TorZillaPrint.html#language
You can check the js code here: https://ghacksuserjs.github.io/TorZillaPrint/js/language.js

Comment 3

[Gary Chen [:xeonchen]]

Attachment: Bug 1486258 - Tests for privacy.spoof_english

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Hi, so I'm not sure what's the goal here.

Our codebase has a single place where we decide on the default locale for the JS context [0], and it doesn't seem to bring much value to attempt to test each API separately.

But the real issue is that the test you're proposing is testing for particular values coming out of the API. This goes against the "best-effort" Intl API model that Mozilla and other industry leaders cultivate.
In other words, no test should assume particular values for particular locales. Trying to hardcode expectations in tests leads to increased burden on anyone trying to update out Intl database because they have to catch all new failures and fix all the tests each time they update.

For that reason we try hard to never test for values. Testing if the returned locale is en-US makes all the sense. Testing that the decimal separator is , for de is not ok.

My suggestion would be to only check if the resolvedOptions.locale matches your expectation between spoofed and not spoofed.

Another option is to just recognize that the only thing that privacy.spoof_english does is flipping another pref "javascript.use_english_sth" and write a test that verifies that we flip it which should be in the browser, and then another test in spidermonkey that verifies that once you flip it the locale is correct.

I don't think either of those tests should live in intl/l10n/tests since they're not related to the code in that module.

[0] https://searchfox.org/mozilla-central/source/js/xpconnect/src/XPCLocale.cpp#131-138

Comment 5

[Gary Chen [:xeonchen]]

Hi Arthur,

Do you think if we only test |Intl.PluralRules().resolvedOptions().locale| is sufficient for the bug?

Comment 6

[Arthur Edelstein [:arthur]]

Hi Gary -- one possibility might be to check each API separately, but not use any hard-coded values. For example, first set the locale to "en-US" and stored the API values. Then switch the locale to "de", enabled privacy.spoof_english and check if the API values are the same.

I understand Zibi's point that there is a single place in the codebase where the default locale is set, but I think it's valuable to check for any unexpected locale leaks that could arise after a refactoring, even if there is some redundancy.

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

I understand Zibi's point that there is a single place in the codebase where the default locale is set, but I think it's valuable to check for any unexpected locale leaks that could arise after a refactoring, even if there is some redundancy.

Yeah, I'm ok with a small leakage test like that. Preferably in form of something very stable (for example, we could take a value for the weekday Monday and test that after we change locale, the string returned out of the API changes or doesn't change).

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1fecce7afe30
Tests for privacy.spoof_english r=zbraniecki

Comment 9

[Arthur Iakab [arthur_iakab]]

Backed out changeset 1fecce7afe30 (bug 1486258) for mochitest failures on test_spoof_english.html CLOSED TREE

Backout revision https://hg.mozilla.org/integration/autoland/rev/4171dda74df83f2300c1be90e5144a7fe7852b70

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=227202219&revision=1fecce7afe30aa5645e6dda54d82c4d9f9cfdf36

Log failures: The failures first started on Test verify https://treeherder.mozilla.org/logviewer.html#?job_id=227198505&repo=autoland

And the migrated to tier 1 https://treeherder.mozilla.org/logviewer.html#?job_id=227198482&repo=autoland

:xeonchen please take a look.

Comment 10

[Gary Chen [:xeonchen]]

Attachment: Bug 1486258 - Tests for privacy.spoof_english;

Test Intl APIs to make sure privacy.spoof_english really works

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/709a25d2a345
Tests for privacy.spoof_english; r=zbraniecki

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/709a25d2a345