Issue 3461 - Fixed: Issue Reporter assumes that RequestEntry.filter is a Filter instance, not a str…

chrome/content/ui/sendReport.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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 806 matching lines @@
 
         for (let i = 0; i < this.needUpdate.length; i++)
           if (Synchronizer.isExecuting(this.needUpdate[i].url))
             return;
 
         FilterNotifier.removeListener(listener);
         E("updateInProgress").hidden = "true";
 
         let filtersRemoved = false;
         let requests = requestsDataSource.origRequests;
         for (let i = 0; i < requests.length; i++)

[Sebastian Noack, 2016/01/04 18:14:52]

Nit: While on it, how about using a for-of loop?

[Wladimir Palant, 2016/01/04 19:23:44]

There is too much to be fixed in this code already, I'm not going to change
unrelated code and risk regressions.

[Sebastian Noack, 2016/01/05 09:20:52]

Fair enough.

-          if (requests[i].filter && !requests[i].filter.subscriptions.filter(s => !s.disabled).length)
+        {
+          if (!requests[i].filter)
+            continue;
+
+          let filter = Filter.fromText(requests[i].filter);
+          if (!filter.subscriptions.filter(s => !s.disabled).length)

[Sebastian Noack, 2016/01/05 09:20:52]

How about |filter.subscriptions.some(s => !s.disabled)|? Or do you consider that
a too risky unrelated change as well?

[Wladimir Palant, 2016/01/05 09:50:08]

This functionality isn't exactly easy to test. But... ok, done.

             filtersRemoved = true;
+        }
 
         if (filtersRemoved)
         {
           // Force the user to reload the page
           E("updateFixedIssue").hidden = false;
           document.documentElement.canAdvance = true;
 
           let nextButton = document.documentElement.getButton("next");
           [nextButton.label, nextButton.accessKey] = Utils.splitLabel(E("updatePage").getAttribute("reloadButtonLabel"));
           document.documentElement.addEventListener("wizardnext", function(event)
@@ skipping 744 matching lines @@
 
 function censorURL(url)
 {
   return url.replace(/([?;&\/#][^?;&\/#]+?=)[^?;&\/#]+/g, "$1*");
 }
 
 function encodeHTML(str)
 {
   return str.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;").replace(/"/g, "&quot;");
 }