Issue 5873 - Show original subscription title in languages table

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 71 matching lines @@
   {
     let access = (item.url || item.text).replace(/'/g, "\\'");
     return function(container)
     {
       return container.querySelector("[data-access='" + access + "']");
     };
   };
 
   Collection.prototype._getItemTitle = function(item, i)
   {
-    if (this.details[i].useSpecialization && item.specialization)
-    {

[ire, 2018/01/04 08:32:52]

NIT: I think this was cleaner without the nested `if`s

[saroyanm, 2018/01/04 21:28:52]

Yes, previously logic was bit different:
Previously, we were defining which attribute value to use, while with new
requirements we have an exceptional case, so I implemented:
if useSpacialization then use it and add English if aplicable.
While previously it was along the lines:
If useSpacialization then use it, if useOriginalTitle then use it.

I think you are right and my approach in general is wrong that why it doesn't
feel to be correct. I should have separated special case handling from core
"collection" functionality.

I did create a special case separate function, so we can specify custom function
for every special "_getItemTitle" case, so it will still fit into the logic of
collections.

I hope this implementation is more clear.

[ire, 2018/01/05 09:23:45]

Ack. Yes it is more clear, thanks!

-      let title = item.specialization;
-      if (item.originalTitle && item.originalTitle.indexOf("+EasyList") >= 0)

[ire, 2018/01/04 08:32:52]

NIT: I don't know if it actually makes a difference, but I always prefer
checking for `> -1` rather than `>= 0`. 

[saroyanm, 2018/01/04 21:28:52]

Done.

-        title += " + " + getMessage("options_english");
-      return title;
-    }
+    if (this.details[i].getTitleFunction)
+      return this.details[i].getTitleFunction(item);
     return item.title || item.url || item.text;
   };
 
   Collection.prototype._sortItems = function()
   {
     this.items.sort((a, b) =>
     {
       // Make sure that Acceptable Ads is always last, since it cannot be
       // disabled, but only be removed. That way it's grouped together with
       // the "Own filter list" which cannot be disabled either at the bottom
@@ skipping 243 matching lines @@
 
   collections.protection = new Collection([
     {
       id: "recommend-protection-list-table"
     }
   ]);
   collections.langs = new Collection([
     {
       id: "blocking-languages-table",
       emptyTexts: ["options_language_empty"],
-      useSpecialization: true
+      getTitleFunction: getLanguageTitle
     }
   ]);
   collections.allLangs = new Collection([
     {
       id: "all-lang-table-add",
       emptyTexts: ["options_dialog_language_other_empty"],
-      useSpecialization: true
+      getTitleFunction: getLanguageTitle
     }
   ]);
   collections.more = new Collection([
     {
       id: "more-list-table",
       setEmptyAction: "hide-more-filters-section",
       removeEmptyAction: "show-more-filters-section"
     }
   ]);
   collections.whitelist = new Collection([
@@ skipping 103 matching lines @@
     if (index >= 0)
       customFilters.splice(index, 1);
 
     updateCustomFiltersUi();
   }
 
   function updateCustomFiltersUi()
   {
     let customFiltersListElement = E("custom-filters-raw");
     customFiltersListElement.value = customFilters.join("\n");
+  }
+
+  function getLanguageTitle(item)
+  {
+    let title = item.specialization;
+    if (item.originalTitle && item.originalTitle.indexOf("+EasyList") > -1)
+      title += " + " + getMessage("options_english");
+    return title;
   }
 
   function loadRecommendations()
   {
     fetch("subscriptions.xml")
       .then((response) =>
       {
         return response.text();
       })
       .then((text) =>
@@ skipping 963 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);
 }