Issue 1668 - Fixed direction on add-on pages for right-to-left scripts

chrome/content/ui/i18n.js

Index: chrome/content/ui/i18n.js
===================================================================
--- a/chrome/content/ui/i18n.js
+++ b/chrome/content/ui/i18n.js
@@ -16,14 +16,13 @@
  */
 
 var i18n;
+var locale;
 
 if (typeof ext != "undefined")
+{
   i18n = ext.i18n;
-else if (typeof chrome != "undefined")
-  // TODO: This check only exist for backwards compatibility, while the Safari
-  // port isn't merged into the adblockpluschrome repo. So this branch should
-  // be removed when the Safari port was merged.
-  i18n = chrome.i18n;
+  locale = ext.i18n.getMessage("@@ui_locale").replace(/_/g, "-");

[Thomas Greiner, 2014/12/09 10:29:04]

What about Safari? We don't handle those predefined messages there yet so we
would need to implement those first in safari/ext/common.js

[Sebastian Noack, 2014/12/09 10:41:13]

We do handle @@ui_locale on Safari.

[Thomas Greiner, 2014/12/09 16:51:56]

Sorry, missed that before. But why not simply use `Utils.appLocale`? Because
that's exactly the same code as used here and you are already using it in the
Firefox-specific code below anyway.

[Sebastian Noack, 2014/12/09 17:40:19]

Because in block.html on Chrome/Opera/Safari there is no require(), and
therefore no Utils.

+}
 else
 {
   // Using Firefox' approach on i18n instead
@@ -84,6 +83,8 @@
       }
     };
   })();
+
+  locale = require("utils").Utils.appLocale;
 }
 
 // Inserts i18n strings into matching elements. Any inner HTML already in the element is
@@ -146,5 +147,14 @@
     return [timeString, d.toLocaleDateString()];
 }
 
-// Fill in the strings as soon as possible
-window.addEventListener("DOMContentLoaded", loadI18nStrings, true);
+function onDOMLoaded()
+{
+  document.documentElement.lang = locale;
+
+  if (locale == "ar" || locale == "he")

[Thomas Greiner, 2014/12/09 10:29:04]

Similarly to above we could use `ext.i18n.getMessage("@@bidi_dir")` to get the
text direction. See
https://developer.chrome.com/extensions/i18n#overview-predefined

[Sebastian Noack, 2014/12/09 10:41:13]

Is there a way to get the direction on Firefox? Otherwise I'd prefer to keep the
check here, instead duplicating it for Firefox and Safari, in the receptive
abstraction code.

[Thomas Greiner, 2014/12/09 16:51:56]

It could be handled the same way as "@@ui_locale" but I guess for now this
should do.

BTW in Firefox there are preferences of the form `intl.uidirection.<locale>` (as
used in https://developer.mozilla.org/en-US/docs/Web/CSS/:-moz-locale-dir(rtl)).
They are only set for the locales "ar", "fa", "he", "ug" and "ur".

A Firefox-specific version would then look like this:

var dir = "ltr";
if (Services.prefs.getPrefType("intl.uidirection." + locale))
  dir = Services.prefs.getCharPref("intl.uidirection." + locale;

[Wladimir Palant, 2014/12/09 16:59:08]

Utils.chromeRegistry.isLocaleRTL("adblockplus") should be better. Internally it
will do exactly the same thing but we don't get to rely on Firefox
implementation details.

[Sebastian Noack, 2014/12/09 17:40:19]

Done. I'm going to implement @@bidi_dir for Safari.

+    document.documentElement.dir = "rtl";
+
+  loadI18nStrings();
+}
+
+window.addEventListener("DOMContentLoaded", onDOMLoaded, true);