Prepared adblockplus for Safari

chrome/content/ui/firstRun.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2013 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 35 matching lines @@
 
   function onDOMLoaded()
   {
     // Show warning if data corruption was detected
     if (typeof backgroundPage != "undefined" && backgroundPage.seenDataCorruption)
     {
       E("dataCorruptionWarning").removeAttribute("hidden");
       setLinks("dataCorruptionWarning", Utils.getDocLink("knownIssuesChrome_filterstorage"));
     }
 
+    // Show warning if Safari < 6
+    var info = require('info');

[Wladimir Palant, 2013/09/10 06:53:46]

Nit: Please use double quotation marks unless there is a reason to use single
quotation marks.

+    if (info.platform == "safari" && info.platformVersion.split(".") < [6])

[Wladimir Palant, 2013/09/10 06:53:46]

This is JavaScript, not Python ;)

Please have a look at the result of [12] < [6]. Arrays and other non-primitive
types aren't comparable directly. This means that JavaScript implicitly converts
them to strings before comparing.

Ideally you could use Services.vc.compare(info.platformVersion, "6.0") here
(requires implementing wrappers to call into the global page as mentioned
above).

[Sebastian Noack, 2013/09/10 10:56:29]

Services.vc.compare is Firefox specific, isn't it? However
info.platformVersion.split(".")[0] < 6 seems to work. Any reasons to don't do it
that way?

[Wladimir Palant, 2013/09/10 11:10:14]

No, we also have our own version comparator implementation in compat.js.
Because the version comparator is the way we compare versions - it works
correctly with complicated version numbers as well. And version comparison is
non-trivial as you've just proved again.

Your new code is also wrong, please look at the result of "12.0".split(".")[0] <
6. Comparing a string to a number implicitly converts the latter to a number. To
do a proper numerical comparison you need an explicit conversion to a number:
parseInt(info.platformVersion.split(".")[0], 10). So, how about using the
standard tool for this job?

[Wladimir Palant, 2013/09/10 11:12:52]

Argh, should be: "implicitly converts the latter to a string".

[Sebastian Noack, 2013/09/10 14:20:57]

Well, "12.0".split(".")[0] < 6, returns false as it should (also in Firefox).
And that is because of, as you just said yourself, the string is converted to a
number during the comparison. I don't get what you mean.
Then I will use it of course. I just assumed that it was Firefox specific and
that we can't use it in other browsers. But if we have  a compatibility
implementation, then it's fine.

[Wladimir Palant, 2013/09/10 14:34:21]

Never mind, my bad. Regardless, the version comparator should be used for
version comparisons.

+      E("legacySafariWarning").removeAttribute("hidden");
+
     // Set up URL
     setLinks("acceptableAdsExplanation", Utils.getDocLink("acceptable_ads_criteria"), openFilters);
 
     shade = E("shade");
     shade.addEventListener("mouseover", scrollPage, false);
     shade.addEventListener("mouseout", stopScroll, false);
 
     // Set up feature buttons linked to subscriptions
     featureSubscriptions.forEach(setToggleSubscriptionButton);
     var filterListener = function(action)
@@ skipping 191 matching lines @@
   {
     var button = E("toggle-" + feature);
     if (isEnabled)
       button.classList.remove("off");
     else
       button.classList.add("off");
   }
 
   document.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 })();