Issue 4167 - getSelectorsForDomain criteria + keys

lib/elemHide.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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 168 matching lines @@
 
     FilterNotifier.emit("elemhideupdate");
   },
 
   _removeFilterKey: function(key, filter)
   {
     let filterKeys = filterKeysBySelector[filter.selector];
     if (filterKeys)
     {
       let index = filterKeys.indexOf(key);
       if (index >= 0)

[kzar, 2016/09/20 18:37:35]

It kind of bugs me how we check index twice, also how we potentially check that
filterKeys.length > 1 even if the index is bigger than 0. I can't see a cleaner
way however :(

       {
         if (filterKeys.length > 1)
         {
           filterKeys.splice(index, 1);
           if (index == 0)
             unconditionalFilterKeys = null;
         }
         else
         {
           delete filterKeysBySelector[filter.selector];
@@ skipping 121 matching lines @@
       let selectors = this.getUnconditionalSelectors();
       unconditionalFilterKeys = [];
       for (let selector of selectors)
         unconditionalFilterKeys.push(filterKeysBySelector[selector][0]);
     }
     return unconditionalFilterKeys.slice();
   },
 
 
   /**
-   * Constant used by getSelectorsForDomain's when matching all selectors.
+   * Constant used by getSelectorsForDomain to return all selectors applying to
+   * a particular hostname.
    */
   ALL_MATCHING: 0,
 
   /**
-   * Constant used by getSelectorsForDomain's when not matching unconditional
-   * selectors.
+   * Constant used by getSelectorsForDomain to exclude selectors which apply to
+   * all websites without exception.
    */
   NO_UNCONDITIONAL: 1,
 
   /**
-   * Constant used by getSelectorsForDomain's when only matching specific
-   * selectors.
+   * Constant used by getSelectorsForDomain to return only selectors for filters
+   * which specifically match the given host name.
    */
   SPECIFIC_ONLY: 2,
 
   /**
-   * Returns a list of all selectors active on a particular domain. Optionally
-   * a list of corresponding filter keys for the selectors can be returned too.
-   * The optional criteria parameter restricts the results, and must have the
-   * value of ALL_MATCHING, NO_UNCONDITIONAL or SPECIFIC_ONLY if provided.

[Wladimir Palant, 2016/09/26 14:56:54]

Nit: You are documenting return value and parameters here, not the function. How
about documenting everything properly rather than inline:

  Determines from the current filter list which selectors should be applied on a
particular host name.

  @param {String} domain
  @param {Number} [criteria]
    One of ALL_MATCHING, NO_UNCONDITIONAL or SPECIFIC_ONLY.
  @param {Boolean} [provideFilterKeys]
    If true, will return a list of corresponding filter keys in addition to
selectors.
  @returns {string[]|Array.<string[]>} List of selectors or an array with two
elements (list of selectors and list of corresponding keys) if provideFilterKeys
is true.

[kzar, 2016/09/27 12:40:22]

Done.

-   */
-  getSelectorsForDomain: function(/**String*/ domain,
-                                  /**Number*/ criteria,
-                                  /**Boolean*/ provideFilterKeys)
+   * Determines from the current filter list which selectors should be applied
+   * on a particular host name. Optionally returns the corresponding filter
+   * keys.
+   * @param {String} domain
+   * @param {Number} [criteria]
+   *   One of the following: ElemHide.ALL_MATCHING, ElemHide.NO_UNCONDITIONAL or
+   *                         ElemHide.SPECIFIC_ONLY.
+   * @param {Boolean} [provideFilterKeys]
+   *   If true, the function will return a list of corresponding filter keys in
+   *   addition to selectors.
+   * @returns {string[]|Array.<string[]>}
+   *   List of selectors or an array with two elements (list of selectors and
+   *   list of corresponding keys) if provideFilterKeys is true.
+   */
+  getSelectorsForDomain: function(domain, criteria, provideFilterKeys)
   {
     let filterKeys = [];
     let selectors = [];
 
     if (typeof criteria == "undefined")
       criteria = ElemHide.ALL_MATCHING;
     if (criteria < ElemHide.NO_UNCONDITIONAL)
     {
       selectors = this.getUnconditionalSelectors();
       if (provideFilterKeys)
@@ skipping 33 matching lines @@
       let nextDot = currentDomain.indexOf(".");
       currentDomain = nextDot == -1 ? "" : currentDomain.substr(nextDot + 1);
     }
 
     if (provideFilterKeys)
       return [selectors, filterKeys];
     else
       return selectors;
   }
 };