Issue 1663 - Various Firefox-related changes of the first-run page

messageResponder.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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 (function(global)
 {
-  if (typeof ext == "undefined")

[Thomas Greiner, 2015/01/07 12:19:55]

Nit: To make it consistent with all other references to `ext` in this file
please change it to `global.ext` here as well

[Sebastian Noack, 2015/01/07 12:23:21]

Why not just |"ext" in global| then?

[Wladimir Palant, 2015/01/07 16:33:10]

Done.

+  if (!global.ext)
     global.ext = require("ext_background");
 
   var Utils = require("utils").Utils;
   var FilterStorage = require("filterStorage").FilterStorage;
   var FilterNotifier = require("filterNotifier").FilterNotifier;
   var defaultMatcher = require("matcher").defaultMatcher;
   var BlockingFilter = require("filterClasses").BlockingFilter;
   var Synchronizer = require("synchronizer").Synchronizer;
 
   var subscriptionClasses = require("subscriptionClasses");
@@ skipping 56 matching lines @@
         });
       }
     }
   };
 
   global.ext.onMessage.addListener(function(message, sender, callback)
   {
     switch (message.type)
     {
       case "app.get":
         if (message.what == "issues")

[Thomas Greiner, 2015/01/07 12:19:55]

There's quite a bunch of ifs now so please turn this into a switch-statement.

[Wladimir Palant, 2015/01/07 16:33:10]

Quite frankly, I consider a nested switch statement absolutely unreadable.

[Thomas Greiner, 2015/01/07 17:31:38]

Since for this particular case we're debating personal preferences, let's leave
it as you wrote it for now.

Generally, how about introducing brackets for switch-statements to our coding
style to make them more legible. It would also eliminate the need to consider
the exception that's specified in Mozilla's coding style: "When you need to
declare a variable in a switch, put the block in braces" (see
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style...)

[Wladimir Palant, 2015/01/07 19:03:39]

While I am inclined to agree, this will unfortunately make switch statements
even more verbose than they already are. And it's still going to require two
indentation levels.

         {
           var info = require("info");
           callback({
             seenDataCorruption: "seenDataCorruption" in global ? global.seenDataCorruption : false,
             filterlistsReinitialized: "filterlistsReinitialized" in global ? global.filterlistsReinitialized : false,
             legacySafariVersion: (info.platform == "safari" && (
                 Services.vc.compare(info.platformVersion, "6.0") < 0 ||   // beforeload breaks websites in Safari 5
                 Services.vc.compare(info.platformVersion, "6.1") == 0 ||  // extensions are broken in 6.1 and 7.0
                 Services.vc.compare(info.platformVersion, "7.0") == 0))
           });
@@ skipping 65 matching lines @@
         }
 
         if (message.filter)
           filters.subscription = message.filter;
         else
           delete filters.subscription;
         break;
     }
   });
 })(this);