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)

[Thomas Greiner, 2014/12/10 10:29:42]

Style nit: I do see why you used underscores here but JavaScript variables
shouldn't contain any underscores to keep it consistent with the rest of our
code.

Same applies to `bidi_dir` below.

[Sebastian Noack, 2014/12/10 10:37:29]

That wasn't an aware decision. It happened rather accidentally when I copied the
variable names from the message names. Sure, that should be camel-case.

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

[Thomas Greiner, 2014/12/10 10:29:42]

This regular expression will match any string due to the `|` at the end. I'd
assume you wanted to make the underscore optional. In this case just replace `|`
with `?`.

[Sebastian Noack, 2014/12/10 10:37:29]

Ouch, I forgot to set parentheses.
Then "ar_" would match while "ar_SA" wouldn't.

[Thomas Greiner, 2014/12/10 11:12:12]

The corrected version still matches "ar_". By adding `.` after `_` you could
ensure that it won't match. Apart from that it looks good.

[Sebastian Noack, 2014/12/10 11:32:06]

Why should we care? We want to match "ar" and "ar_SA". "ar_" is a theoretical
case we can ignore. And even if it occurs, it can still be considered a variant
of Arabic, where we have to apply right-to-left.

[Thomas Greiner, 2014/12/10 12:06:02]

I can agree with that line of argument.

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