Issue 1678 - Added bidi support for Safari

safari/ext/common.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 86 matching lines @@
       candidates.push(language + "_" + region);
 
     candidates.push(language);
 
     if (candidates.indexOf(defaultLocale) == -1)
       candidates.push(defaultLocale);
 
     return candidates;
   };
 
-  var initCatalog = function(ui_locale)
-  {
-    var bidi_dir = (ui_locale == "ar" ||
-                    ui_locale == "fa" ||
-                    ui_locale == "he" ||
-                    ui_locale == "ug" ||
-                    ui_locale == "ur") ? "rtl" : "ltr";
-
-    return {
-      __proto__: null,
-
-      "@@ui_locale": [ui_locale, []],
-      "@@bidi_dir":  [bidi_dir,  []]
-    };

[Wladimir Palant, 2014/12/09 18:59:09]

Nit: Please consider using Object.create(null) instead of {__proto__:null}, the
latter being non-standard.

[Sebastian Noack, 2014/12/09 19:30:16]

I haven't seen through your policy for unrelated changes yet. ;)

Done.

[Wladimir Palant, 2014/12/09 19:55:37]

The policy is very simple - fix the code when you touch it, unless it will
complicate the review significantly. :)

+  var initCatalog = function(uiLocale)
+  {
+    var bidiDir = /^(ar|fa|he|ug|ur)(_|$)/.test(uiLocale) ? "rtl" : "ltr";

[Wladimir Palant, 2014/12/11 16:52:15]

I only realized now that we are looking at UI locale here rather than our
locale. What will happen if the UI locale is "ur" and we don't have that
translation? I guess that the user will see the fallback to English yet rendered
as RTL. Given that his browser UI is RTL, this might even make sense...

But I guess that this is a purely theoretical discussion. Sebastian mentioned
that Safari only supports ar and he, we have both of them covered.

[Sebastian Noack, 2014/12/11 17:03:05]

To be honest I didn't consider that case and agree that this code is
theoretically wrong, in terms that it considers the language of the browser,
instead the language of the translation used.

However, as you say, currently the only right-to-left languages supported by OS
X are Arabic and Hebrew. So this case won't happen in practice. Also note that
considering the language of the translation would require some additional
complexity since translations are loaded lazily on first use.

+    var catalog = Object.create(null);
+
+    catalog["@@ui_locale"] = [uiLocale, []];
+    catalog["@@bidi_dir" ] = [bidiDir,  []];
+
+    return catalog;
   };
 
   var locales = getLocaleCandidates();
   var catalog = initCatalog(locales[0]);
 
   var replacePlaceholder = function(text, placeholder, content)
   {
     return text.split("$" + placeholder + "$").join(content || "");
   };
 
@@ skipping 68 matching lines @@
   };
 
 
   /* Utils */
 
   ext.getURL = function(path)
   {
     return safari.extension.baseURI + path;
   };
 })();