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

RESOLVED FIXED in Firefox 67

Status

()

defect
P2
normal
RESOLVED FIXED
10 months ago
4 months ago

People

(Reporter: arthur, Assigned: xeonchen)

Tracking

(Blocks 1 bug)

unspecified
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

(Whiteboard: [tor 26611][fingerprinting][fp-triaged])

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

10 months ago
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.)
Reporter

Comment 1

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

Updated

10 months ago
Component: Internationalization → JavaScript: Internationalization API

Updated

7 months ago
Assignee: nobody → xeonchen
Priority: -- → P2
Whiteboard: [tor 26611] → [tor 26611][fingerprinting][fp-triaged]

Comment 2

5 months ago

(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

Assignee

Comment 3

5 months ago

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

Attachment #9040189 - Attachment is obsolete: true
Assignee

Comment 5

5 months ago

Hi Arthur,

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

Assignee

Updated

5 months ago
Flags: needinfo?(arthur)
Reporter

Comment 6

4 months ago

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.

Flags: needinfo?(arthur)

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).

Attachment #9040189 - Attachment is obsolete: false

Comment 8

4 months ago
Pushed by xeonchen@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/1fecce7afe30
Tests for privacy.spoof_english r=zbraniecki
Assignee

Comment 10

4 months ago

Test Intl APIs to make sure privacy.spoof_english really works

Assignee

Updated

4 months ago
Attachment #9040189 - Attachment is obsolete: true
Flags: needinfo?(xeonchen)

Comment 11

4 months ago
Pushed by xeonchen@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/709a25d2a345
Tests for privacy.spoof_english; r=zbraniecki

Comment 12

4 months ago
bugherder
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.