Issue 1678 - Added bidi support for Safari

safari/ext/common.js

Index: safari/ext/common.js
===================================================================
--- a/safari/ext/common.js
+++ b/safari/ext/common.js
@@ -104,8 +104,19 @@
     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)
   {