Issue 5706 - Finish custom filter list dialog

new-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 661 matching lines @@
         });
         break;
       case "update-subscription":
         ext.backgroundPage.sendMessage({
           type: "subscriptions.update",
           url: findParentData(element, "access", false)
         });
         break;
       case "validate-import-subscription":
         let form = findParentData(element, "validation", true);
-        if (form && form.checkValidity())
-        {
-          addEnableSubscription(E("import-list-url").value, 
+        if (!form)
+          return;
+
+        if (form.checkValidity())
+        {
+          addEnableSubscription(E("import-list-url").value,
             E("import-list-title").value);
           form.reset();
           closeDialog();
+        }
+        else
+        {
+          form.querySelector(":invalid").focus();
         }

[ire, 2017/09/20 14:53:12]

I'm not sure why we need the eventListener on the forms you added below, when we
can check for the form being invalid here in the `else` part of this statement.
The only issue I see with that is in calling `e.preventDefault()`, since the
event is not passed to this `exec` function (which it could be added).

The reason I'm bringing this up is because it may be unnecessary to add event
listeners to *every* `[data-validation]` form, when there is already an event
listener added for the submit of the form (via the click of the submit button in
this case).

Although your method has the benefit of being extensible to other forms that may
need validation, the logic behind that could be extracted without the need to
add the event listeners to every form. 

[saroyanm, 2017/09/21 19:07:54]

You are absolutely right. The reason was because I wanted to prevent default on
all forms submission, while all other code should be here.
Not to complicate this with the submission issue, I'll not use type="submit".
Because if we would like to handle this on submission we probably need to create
a submit handling function, where we might want to get action from the form
element and I think that can be tackled here ->
https://issues.adblockplus.org/ticket/5251 .
Anyway, done :) 

[ire, 2017/09/22 08:28:53]

Got it :)

As long as the button will be changed to a submit in #5251, that's fine by me!

         break;
     }
   }
 
   function setCustomFiltersView(mode)
   {
     let customFiltersElement = E("custom-filters-raw");
     updateCustomFiltersUi();
     if (mode == "read")
     {
@@ skipping 136 matching lines @@
     {
       E("abp-version").textContent = getMessage("options_dialog_about_version",
         [addonVersion]);
     });
 
     updateTooltips();
 
     // Initialize interactive UI elements
     document.body.addEventListener("click", onClick, false);
     document.body.addEventListener("keyup", onKeyUp, false);
-    document.body.addEventListener("submit", (e) =>

[ire, 2017/09/20 14:53:13]

The "submit" event listener is only relevant to forms. I don't think this is
necessary or does anything

[saroyanm, 2017/09/21 19:07:54]

Well spotted, I Forgot to delete this.
Done.

-    {
-      e.preventDefault();
-    }, false);
-
-    // Overwrite form input validation
-    let forms = document.querySelectorAll("[data-validation]");
-    for (let form of forms)
-    {
-      form.addEventListener("invalid", (e) =>
-      {
-        e.preventDefault();
-        form.querySelector(":invalid").focus();

[saroyanm, 2017/09/19 18:53:02]

I need to select first invalid input

[ire, 2017/09/20 14:53:13]

Acknowledged.

-      }, true);
-    }
-
     let exampleValue = getMessage("options_whitelist_placeholder_example",
       ["www.example.com"]);
     E("whitelisting-textbox").setAttribute("placeholder", exampleValue);
     E("whitelisting-textbox").addEventListener("keyup", (e) =>
     {
       E("whitelisting-add-button").disabled = !e.target.value;
     }, false);
 
 
     getDocLink("contribute", (link) =>
@@ skipping 488 matching lines @@
   });
   ext.backgroundPage.sendMessage({
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
              "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
   window.addEventListener("hashchange", onHashChange, false);
 }