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

chrome/content/ui/sendReport.js

Index: chrome/content/ui/sendReport.js
===================================================================
--- a/chrome/content/ui/sendReport.js
+++ b/chrome/content/ui/sendReport.js
@@ -820,18 +820,24 @@ var subscriptionUpdateDataSource =
             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");