Issue 5549 - Implement missing error handlings for custom filter lists

desktop-options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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 529 matching lines @@
         if (returnElement)
           return element;
         return element.getAttribute("data-" + dataName);
       }
 
       element = element.parentElement;
     }
     return null;
   }
 
-  function sendMessageHandleErrors(message, onSuccess)
+  function sendMessageHandleErrors(message, callback)
   {
     browser.runtime.sendMessage(message, (errors) =>
     {
-      if (errors.length > 0)
-        alert(errors.join("\n"));
-      else if (onSuccess)
-        onSuccess();
+      if (callback)
+      {
+        if (errors.length > 0)
+          callback(errors);
+        else
+          callback();
+      }
     });
   }
 
   function switchTab(id)
   {
     location.hash = id;
   }
 
   function execAction(action, element)
   {
@@ skipping 73 matching lines @@
           text: findParentData(element, "access", false)
         });
         break;
       case "remove-subscription":
         browser.runtime.sendMessage({
           type: "subscriptions.remove",
           url: findParentData(element, "access", false)
         });
         break;
       case "save-custom-filters":
+        const filters = E("custom-filters-raw").value;
         sendMessageHandleErrors({
           type: "filters.importRaw",
-          text: E("custom-filters-raw").value,
+          text: filters,
           removeExisting: true
         },
-        () =>
+        (errors) =>
         {
-          setCustomFiltersView("read");
+          if (errors)
+          {
+            E("custom-filters").classList.add("warning");
+            // The current error does not contain info about the line
+            // that generated such error.
+            // Whenever the error object will pass the bad filter
+            // within its properties, this split should be removed.
+            const lines = filters.split(/\n+|\n\s+\n/);
+            const messages = errors.map(error => lines[error.lineno - 1]);
+            const editFilters = E("custom-filters-error");
+            for (const message of messages)
+            {
+              const li = document.createElement("li");
+              editFilters.appendChild(li).textContent = message;

[saroyanm, 2018/01/30 12:03:44]

The Error keep being added to the error container.

+            }
+            if (errors.length > 5)
+              editFilters.classList.add("many");
+          }
+          else
+          {
+            setCustomFiltersView("read");
+          }
         });
         break;
       case "show-more-filters-section":
         E("more-filters").setAttribute("aria-hidden", false);
         break;
       case "switch-acceptable-ads":
         let value = element.value || element.dataset.value;
         // User check the checkbox
         let shouldCheck = element.getAttribute("aria-checked") != "true";
         let installAcceptableAds = false;
@@ skipping 79 matching lines @@
         break;
     }
   }
 
   function setCustomFiltersView(mode)
   {
     let customFiltersElement = E("custom-filters-raw");
     updateCustomFiltersUi();
     if (mode == "read")
     {
+      E("custom-filters").classList.remove("warning");
+      const editFilters = E("custom-filters-error");
+      editFilters.textContent = "";
+      editFilters.classList.remove("many");
       customFiltersElement.disabled = true;
       if (!customFiltersElement.value)
       {
         setCustomFiltersView("empty");
         return;
       }
     }
     else if (mode == "write")
     {
       customFiltersElement.disabled = false;
@@ skipping 143 matching lines @@
 
     // General tab
     getDocLink("contribute", (link) =>
     {
       E("contribute").href = link;
     });
     getDocLink("acceptable_ads_criteria", (link) =>
     {
       setLinks("enable-acceptable-ads-description", link);
     });
-    setElementText(E("tracking-warning-1"), "options_tracking_warning_1", 
+    setElementText(E("tracking-warning-1"), "options_tracking_warning_1",
       [getMessage("common_feature_privacy_title"),
-      getMessage("options_acceptableAds_ads_label")]);
-    setElementText(E("tracking-warning-3"), "options_tracking_warning_3", 
+       getMessage("options_acceptableAds_ads_label")]);
+    setElementText(E("tracking-warning-3"), "options_tracking_warning_3",
       [getMessage("options_acceptableAds_privacy_label")]);
 
     getDocLink("privacy_friendly_ads", (link) =>
     {
       E("enable-acceptable-ads-privacy-description").href = link;
     });
     getDocLink("adblock_plus_{browser}_dnt", url =>
     {
       setLinks("dnt", url);
     });
@@ skipping 21 matching lines @@
       type: "app.get",
       what: "features"
     },
     (features) =>
     {
       hidePref("show_devtools_panel", !features.devToolsPanel);
     });
 
     getDocLink("filterdoc", (link) =>
     {
-      E("link-filters").setAttribute("href", link);
+      E("link-filters-1").setAttribute("href", link);
+      E("link-filters-2").setAttribute("href", link);
     });
 
     getDocLink("subscriptions", (link) =>
     {
       E("filter-lists-learn-more").setAttribute("href", link);
     });
 
     E("custom-filters-raw").setAttribute("placeholder",
       getMessage("options_customFilters_edit_placeholder", ["/ads/track/*"]));
 
@@ skipping 311 matching lines @@
         setPrivacyConflict();
         break;
       case "downloading":
       case "downloadStatus":
       case "homepage":
       case "lastDownload":
       case "title":
         updateSubscription(subscription);
         break;
       case "added":
-        let {url, recommended} = subscription;
+        let {url} = subscription;
         // Handle custom subscription
         if (/^~user/.test(url))
         {
           loadCustomFilters(subscription.filters);
           return;
         }
         else if (url in subscriptionsMap)
           updateSubscription(subscription);
         else
           addSubscription(subscription);
@@ skipping 160 matching lines @@
   });
   browser.runtime.sendMessage({
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
              "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
   window.addEventListener("hashchange", onHashChange, false);
 }