Bug 1673054 - Migrate uses of intl.uidirection to intl.l10n.pseudo; r=Gijs,zbraniecki

This also removes pref overrides from methods like LocaleService::IsLocaleRTL or
IntlService.getLocaleInfo, because it doesn't really make sense to override the
result of checking an arbitrary locale, the relevant use case is overriding the
result for the current app locale.

Removal of the intl.uidirection pref completely will be done in a separate bug.

Differential Revision: https://phabricator.services.mozilla.com/D96235
This commit is contained in:
Dan Minor
2020-11-09 15:33:39 +00:00
parent 1fce1934d1
commit e19ee18311
22 changed files with 38 additions and 88 deletions

View File

@@ -283,7 +283,7 @@ add_task(async function testArrowsInPanelMultiView() {
// Test that right/left arrows move in the expected direction for RTL locales. // Test that right/left arrows move in the expected direction for RTL locales.
add_task(async function testArrowsRtl() { add_task(async function testArrowsRtl() {
await SpecialPowers.pushPrefEnv({ set: [["intl.uidirection", 1]] }); await SpecialPowers.pushPrefEnv({ set: [["intl.l10n.pseudo", "bidi"]] });
// window.RTL_UI doesn't update in existing windows when this pref is changed, // window.RTL_UI doesn't update in existing windows when this pref is changed,
// so we need to test in a new window. // so we need to test in a new window.
let win = await BrowserTestUtils.openNewBrowserWindow(); let win = await BrowserTestUtils.openNewBrowserWindow();

View File

@@ -94,12 +94,10 @@ add_task(async function test_sidebarpanels_click() {
for (let test of tests) { for (let test of tests) {
gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser); gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
await pushPref("intl.uidirection", 0);
info("Running " + test.desc + " in LTR mode"); info("Running " + test.desc + " in LTR mode");
await testPlacesPanel(test); await testPlacesPanel(test);
await popPref();
await pushPref("intl.uidirection", 1); await pushPref("intl.l10n.pseudo", "bidi");
info("Running " + test.desc + " in RTL mode"); info("Running " + test.desc + " in RTL mode");
await testPlacesPanel(test); await testPlacesPanel(test);
await popPref(); await popPref();

View File

@@ -100,6 +100,7 @@ SyncedTabsDeckComponent.prototype = {
// Add app locale change support for HTML sidebar // Add app locale change support for HTML sidebar
Services.obs.addObserver(this, "intl:app-locales-changed"); Services.obs.addObserver(this, "intl:app-locales-changed");
Services.prefs.addObserver("intl.uidirection", this); Services.prefs.addObserver("intl.uidirection", this);
Services.prefs.addObserver("intl.l10n.pseudo", this);
this.updateDir(); this.updateDir();
// Go ahead and trigger sync // Go ahead and trigger sync
@@ -125,6 +126,7 @@ SyncedTabsDeckComponent.prototype = {
Services.obs.removeObserver(this, UIState.ON_UPDATE); Services.obs.removeObserver(this, UIState.ON_UPDATE);
Services.obs.removeObserver(this, "intl:app-locales-changed"); Services.obs.removeObserver(this, "intl:app-locales-changed");
Services.prefs.removeObserver("intl.uidirection", this); Services.prefs.removeObserver("intl.uidirection", this);
Services.prefs.removeObserver("intl.l10n.pseudo", this);
this._deckView.destroy(); this._deckView.destroy();
}, },
@@ -141,7 +143,7 @@ SyncedTabsDeckComponent.prototype = {
this.updateDir(); this.updateDir();
break; break;
case "nsPref:changed": case "nsPref:changed":
if (data == "intl.uidirection") { if (data == "intl.uidirection" || data == "intl.l10n.pseudo") {
this.updateDir(); this.updateDir();
} }
break; break;

View File

@@ -165,7 +165,7 @@ add_task(async function testObserver() {
"locale change triggers UI direction update" "locale change triggers UI direction update"
); );
Services.prefs.setIntPref("intl.uidirection", 1); Services.prefs.setStringPref("intl.l10n.pseudo", "bidi");
Assert.equal( Assert.equal(
component.updateDir.callCount, component.updateDir.callCount,

View File

@@ -44,7 +44,6 @@ interface nsIChromeRegistry : nsISupports
interface nsIXULChromeRegistry : nsIChromeRegistry interface nsIXULChromeRegistry : nsIChromeRegistry
{ {
// Get whether the default writing direction of the locale is RTL // Get whether the default writing direction of the locale is RTL
// (or may be overridden by intl.uidirection pref)
boolean isLocaleRTL(in ACString package); boolean isLocaleRTL(in ACString package);
/** /**

View File

@@ -13,7 +13,7 @@ requestLongerTimeout(4);
// Test that DevTools panels are rendered in "rtl" (right-to-left) in the Browser Toolbox. // Test that DevTools panels are rendered in "rtl" (right-to-left) in the Browser Toolbox.
add_task(async function() { add_task(async function() {
await pushPref("intl.uidirection", 1); await pushPref("intl.l10n.pseudo", "bidi");
const ToolboxTask = await initBrowserToolboxTask(); const ToolboxTask = await initBrowserToolboxTask();
await ToolboxTask.importFunctions({}); await ToolboxTask.importFunctions({});

View File

@@ -12,7 +12,7 @@ add_task(async function() {
CHROME_URL_ROOT + "current-time-scrubber_head.js", CHROME_URL_ROOT + "current-time-scrubber_head.js",
this this
); );
await pushPref("intl.uidirection", 1); await pushPref("intl.l10n.pseudo", "bidi");
// eslint-disable-next-line no-undef // eslint-disable-next-line no-undef
await testCurrentTimeScrubber(true); await testCurrentTimeScrubber(true);
}); });

View File

@@ -8,7 +8,7 @@ add_task(async function() {
CHROME_URL_ROOT + "keyframes-graph_keyframe-marker_head.js", CHROME_URL_ROOT + "keyframes-graph_keyframe-marker_head.js",
this this
); );
await pushPref("intl.uidirection", 1); await pushPref("intl.l10n.pseudo", "bidi");
// eslint-disable-next-line no-undef // eslint-disable-next-line no-undef
await testKeyframesGraphKeyframesMarker(); await testKeyframesGraphKeyframesMarker();
}); });

View File

@@ -8,7 +8,7 @@ add_task(async function() {
CHROME_URL_ROOT + "summary-graph_delay-sign_head.js", CHROME_URL_ROOT + "summary-graph_delay-sign_head.js",
this this
); );
await pushPref("intl.uidirection", 1); await pushPref("intl.l10n.pseudo", "bidi");
// eslint-disable-next-line no-undef // eslint-disable-next-line no-undef
await testSummaryGraphDelaySign(); await testSummaryGraphDelaySign();
}); });

View File

@@ -10,6 +10,6 @@ add_task(async function() {
CHROME_URL_ROOT + "summary-graph_end-delay-sign_head.js", CHROME_URL_ROOT + "summary-graph_end-delay-sign_head.js",
this this
); );
await pushPref("intl.uidirection", 1); await pushPref("intl.l10n.pseudo", "bidi");
await testSummaryGraphEndDelaySign(); await testSummaryGraphEndDelaySign();
}); });

View File

@@ -46,11 +46,11 @@ add_task(async function() {
await inspectorResized; await inspectorResized;
info("Testing transitions ltr"); info("Testing transitions ltr");
await pushPref("intl.uidirection", 0); await pushPref("intl.l10n.pseudo", "");
await testBreadcrumbTransitions(hostWindow, inspector); await testBreadcrumbTransitions(hostWindow, inspector);
info("Testing transitions rtl"); info("Testing transitions rtl");
await pushPref("intl.uidirection", 1); await pushPref("intl.l10n.pseudo", "bidi");
await testBreadcrumbTransitions(hostWindow, inspector); await testBreadcrumbTransitions(hostWindow, inspector);
hostWindow.resizeTo(originalWidth, originalHeight); hostWindow.resizeTo(originalWidth, originalHeight);

View File

@@ -13,7 +13,11 @@ add_task(async function() {
}); });
async function testForGivenDir(dir) { async function testForGivenDir(dir) {
await pushPref("intl.uidirection", dir === "rtl" ? 1 : -1); if (dir === "rtl") {
await pushPref("intl.l10n.pseudo", "bidi");
} else {
await pushPref("intl.l10n.pseudo", "");
}
// Reset visibleColumns so we only get the default ones // Reset visibleColumns so we only get the default ones
// and not all that are set in head.js // and not all that are set in head.js

View File

@@ -110,8 +110,6 @@ The appropriate value for the `dir` attribute will then be set when the toolbox
The recommended workflow to test RTL on DevTools is to use the [Force RTL extension](https://addons.mozilla.org/en-US/firefox/addon/force-rtl/). After changing the direction using Force RTL, you should restart DevTools to make sure all modules apply the new direction. A future version of Force RTL will be able to update dynamically all DevTools documents.<!--TODO: update when the fate of this addon/webextension is known--> The recommended workflow to test RTL on DevTools is to use the [Force RTL extension](https://addons.mozilla.org/en-US/firefox/addon/force-rtl/). After changing the direction using Force RTL, you should restart DevTools to make sure all modules apply the new direction. A future version of Force RTL will be able to update dynamically all DevTools documents.<!--TODO: update when the fate of this addon/webextension is known-->
Going to `about:config` and setting `intl.uidirection.en` to rtl is not recommended, and will always require to re-open DevTools to have any impact.
## Toggles ## Toggles
Sometimes you have a style that you want to turn on and off. For example a tree twisty (a expand-collapse arrow), a tab background, etc. Sometimes you have a style that you want to turn on and off. For example a tree twisty (a expand-collapse arrow), a tab background, etc.

View File

@@ -621,9 +621,7 @@ The three classes of potential problems that this can help with are:
- Bidi adaptation. - Bidi adaptation.
For many developers, testing the UI in right-to-left mode is hard. Mozilla For many developers, testing the UI in right-to-left mode is hard.
offers a pref :js:`intl.uidirection` which switches the direction of the layout,
but that doesn't expose problems related to right-to-left text.
Pseudolocalization shows how a right-to-left locale will look like. Pseudolocalization shows how a right-to-left locale will look like.
To turn on pseudolocalization, add a new string pref :js:`intl.l10n.pseudo` and To turn on pseudolocalization, add a new string pref :js:`intl.l10n.pseudo` and

View File

@@ -235,6 +235,10 @@ void LocaleService::LocalesChanged() {
} }
bool LocaleService::IsLocaleRTL(const nsACString& aLocale) { bool LocaleService::IsLocaleRTL(const nsACString& aLocale) {
return unic_langid_is_rtl(&aLocale);
}
bool LocaleService::IsAppLocaleRTL() {
// First, let's check if there's a manual override // First, let's check if there's a manual override
// preference for directionality set. // preference for directionality set.
int pref = Preferences::GetInt("intl.uidirection", -1); int pref = Preferences::GetInt("intl.uidirection", -1);
@@ -242,11 +246,7 @@ bool LocaleService::IsLocaleRTL(const nsACString& aLocale) {
return (pref > 0); return (pref > 0);
} }
return unic_langid_is_rtl(&aLocale); // Next, check if there is a pseudo locale `bidi` set.
}
bool LocaleService::IsAppLocaleRTL() {
// First, check if there is a pseudo locale `bidi` set.
nsAutoCString pseudoLocale; nsAutoCString pseudoLocale;
if (NS_SUCCEEDED(Preferences::GetCString("intl.l10n.pseudo", pseudoLocale))) { if (NS_SUCCEEDED(Preferences::GetCString("intl.l10n.pseudo", pseudoLocale))) {
if (pseudoLocale.EqualsLiteral("bidi")) { if (pseudoLocale.EqualsLiteral("bidi")) {

View File

@@ -157,8 +157,6 @@ class LocaleService final : public mozILocaleService,
/** /**
* Returns whether the locale is RTL. * Returns whether the locale is RTL.
*
* This method respects the `intl.uidirection` pref override.
*/ */
static bool IsLocaleRTL(const nsACString& aLocale); static bool IsLocaleRTL(const nsACString& aLocale);

View File

@@ -142,7 +142,13 @@ TEST(Intl_Locale_LocaleService, GetDefaultLocale)
TEST(Intl_Locale_LocaleService, IsAppLocaleRTL) TEST(Intl_Locale_LocaleService, IsAppLocaleRTL)
{ {
// For now we can only test if the method doesn't crash. mozilla::Preferences::SetCString("intl.l10n.pseudo", "bidi");
LocaleService::GetInstance()->IsAppLocaleRTL(); ASSERT_TRUE(LocaleService::GetInstance()->IsAppLocaleRTL());
ASSERT_TRUE(true); mozilla::Preferences::ClearUser("intl.l10n.pseudo");
mozilla::Preferences::SetInt("intl.uidirection", 0);
ASSERT_FALSE(LocaleService::GetInstance()->IsAppLocaleRTL());
mozilla::Preferences::SetInt("intl.uidirection", 1);
ASSERT_TRUE(LocaleService::GetInstance()->IsAppLocaleRTL());
mozilla::Preferences::SetInt("intl.uidirection", -1);
} }

View File

@@ -152,7 +152,6 @@ add_test(function test_isAppLocaleRTL_pseudo() {
localeService.availableLocales = ["en-US"]; localeService.availableLocales = ["en-US"];
localeService.requestedLocales = ["en-US"]; localeService.requestedLocales = ["en-US"];
Services.prefs.setIntPref("intl.uidirection", -1);
Services.prefs.setCharPref("intl.l10n.pseudo", ""); Services.prefs.setCharPref("intl.l10n.pseudo", "");
Assert.ok(localeService.isAppLocaleRTL === false); Assert.ok(localeService.isAppLocaleRTL === false);

View File

@@ -161,14 +161,14 @@ async function test_i18n_css(options = {}) {
// We don't currently have a good way to mock this. // We don't currently have a good way to mock this.
if (false) { if (false) {
const DIR = "intl.uidirection"; const DIR = "intl.l10n.pseudo";
// We don't wind up actually switching the chrome registry locale, since we // We don't wind up actually switching the chrome registry locale, since we
// don't have a chrome package for Hebrew. So just override it, and force // don't have a chrome package for Hebrew. So just override it, and force
// RTL directionality. // RTL directionality.
const origReqLocales = Services.locale.requestedLocales; const origReqLocales = Services.locale.requestedLocales;
Services.locale.requestedLocales = ["he"]; Services.locale.requestedLocales = ["he"];
Preferences.set(DIR, 1); Preferences.set(DIR, "bidi");
css = await fetch(baseURL + "locale.css"); css = await fetch(baseURL + "locale.css");
equal( equal(

View File

@@ -3,9 +3,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
const { XPCOMUtils } = ChromeUtils.import(
"resource://gre/modules/XPCOMUtils.jsm"
);
const mozIntlHelper = Cc["@mozilla.org/mozintlhelper;1"].getService( const mozIntlHelper = Cc["@mozilla.org/mozintlhelper;1"].getService(
Ci.mozIMozIntlHelper Ci.mozIMozIntlHelper
@@ -768,20 +765,6 @@ class MozIntl {
constructor() { constructor() {
this._cache = {}; this._cache = {};
Services.obs.addObserver(this, "intl:app-locales-changed", true); Services.obs.addObserver(this, "intl:app-locales-changed", true);
XPCOMUtils.defineLazyPreferenceGetter(
this,
"_forcedDir",
"intl.uidirection",
-1,
() => this.observe()
);
XPCOMUtils.defineLazyPreferenceGetter(
this,
"_pseudo",
"intl.l10n.pseudo",
"",
() => this.observe()
);
} }
observe() { observe() {
@@ -810,13 +793,7 @@ class MozIntl {
mozIntlHelper.addGetLocaleInfo(this._cache); mozIntlHelper.addGetLocaleInfo(this._cache);
} }
let info = this._cache.getLocaleInfo(getLocales(locales), ...args); return this._cache.getLocaleInfo(getLocales(locales), ...args);
if (this._pseudo == "bidi") {
info.direction = "rtl";
} else if (this._forcedDir != -1) {
info.direction = this._forcedDir == 1 ? "rtl" : "ltr";
}
return info;
} }
getAvailableLocaleDisplayNames(type) { getAvailableLocaleDisplayNames(type) {

View File

@@ -7,7 +7,6 @@ function run_test() {
test_methods_presence(); test_methods_presence();
test_methods_calling(); test_methods_calling();
test_constructors(); test_constructors();
test_direction_adaptation();
test_rtf_formatBestUnit(); test_rtf_formatBestUnit();
ok(true); ok(true);
@@ -59,34 +58,6 @@ function test_constructors() {
}); });
} }
function test_direction_adaptation() {
{
let { direction } = Services.intl.getLocaleInfo("en-US");
equal(direction, "ltr", "Should get ltr for English.");
}
Services.prefs.setIntPref("intl.uidirection", 1);
{
let { direction } = Services.intl.getLocaleInfo("en-US");
equal(direction, "rtl", "Should now get rtl for English.");
}
Services.prefs.clearUserPref("intl.uidirection");
{
let { direction } = Services.intl.getLocaleInfo("en-US");
equal(direction, "ltr", "Should get ltr for English again.");
}
Services.prefs.setCharPref("intl.l10n.pseudo", "bidi");
{
let { direction } = Services.intl.getLocaleInfo("en-US");
equal(direction, "rtl", "Should get rtl after enabling pseudolocale.");
}
Services.prefs.clearUserPref("intl.l10n.pseudo");
{
let { direction } = Services.intl.getLocaleInfo("en-US");
equal(direction, "ltr", "Should finally have ltr for English again.");
}
}
function testRTFBestUnit(anchor, value, expected) { function testRTFBestUnit(anchor, value, expected) {
let rtf = new Services.intl.RelativeTimeFormat("en-US"); let rtf = new Services.intl.RelativeTimeFormat("en-US");
deepEqual(rtf.formatBestUnit(new Date(value), { now: anchor }), expected); deepEqual(rtf.formatBestUnit(new Date(value), { now: anchor }), expected);

View File

@@ -43,7 +43,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=348233
SimpleTest.waitForExplicitFinish(); SimpleTest.waitForExplicitFinish();
let prefs = SpecialPowers.Services.prefs.nsIPrefBranch; let prefs = SpecialPowers.Services.prefs.nsIPrefBranch;
prefs.setIntPref("intl.uidirection", 1); prefs.setCharPref("intl.l10n.pseudo", "bidi");
let rootDir = getRootDirectory(window.location.href); let rootDir = getRootDirectory(window.location.href);
let manifest = rootDir + "rtlchrome/rtl.manifest"; let manifest = rootDir + "rtlchrome/rtl.manifest";
@@ -76,7 +76,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=348233
is(frame.contentDocument.body.dir, "rtl", "file:// listings should be RTL in RTL locales"); is(frame.contentDocument.body.dir, "rtl", "file:// listings should be RTL in RTL locales");
cleanupFunc(); cleanupFunc();
prefs.clearUserPref("intl.uidirection"); prefs.clearUserPref("intl.l10n.pseudo");
SimpleTest.finish(); SimpleTest.finish();
}); });
document.documentElement.appendChild(frame); document.documentElement.appendChild(frame);