Issue 5808 - Easyprivacy URL is being shown instead of the title

messageResponder.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 125 matching lines @@
   {
     let listenerFilters = changeListeners.get(page);
     if (!listenerFilters)
     {
       listenerFilters = Object.create(null);
       changeListeners.set(page, listenerFilters);
     }
     return listenerFilters;
   }
 
+  function addSubscription(subscription, message)

[Thomas Greiner, 2017/09/29 16:39:18]

Detail: The name "message" here is quite generic. For this function the argument
only provides subscription property values so renaming it to something like
"props" would clarify what data we expect to find in it.

[saroyanm, 2017/09/29 16:58:56]

Done.

+  {
+    subscription.disabled = false;
+    if ("title" in message)

[saroyanm, 2017/09/29 14:35:17]

Lack of this check were causing the unexpected behavior, but still can't say for
sure why the URL were assigned to the title, we are not assigning the URL to the
title as in the current/old option page.

[Thomas Greiner, 2017/09/29 16:39:18]

The URL is used as a fallback whenever there's no title (see
https://hg.adblockplus.org/adblockplusui/file/tip/new-options.js#l89) so the
title seems to have mistakenly been overwritten with either `undefined`, `null`
or `""` which caused the URL to show up.

[saroyanm, 2017/09/29 16:58:56]

I see, thanks for the info.

+      subscription.title = message.title;
+    if ("homepage" in message)
+      subscription.homepage = message.homepage;
+
+    FilterStorage.addSubscription(subscription);
+    if (subscription instanceof DownloadableSubscription &&

[saroyanm, 2017/09/29 14:35:17]

Previously we were not checking if it's a DownloadableSubscription in "toggle
message", I think it wasn't intentional, but will value your comment here.

[Thomas Greiner, 2017/09/29 16:39:18]

Acknowledged.

+        !subscription.lastDownload)
+      Synchronizer.execute(subscription);
+  }
+
   port.on("app.get", (message, sender) =>
   {
     if (message.what == "issues")
     {
       let subscriptionInit = require("subscriptionInit");
       let result = subscriptionInit ? subscriptionInit.reinitialized : false;
       return {filterlistsReinitialized: result};
     }
 
     if (message.what == "doclink")
@@ skipping 168 matching lines @@
   {
     if (message.key == "notifications_ignoredcategories")
       return NotificationStorage.toggleIgnoreCategory("*");
 
     return Prefs[message.key] = !Prefs[message.key];
   });
 
   port.on("subscriptions.add", (message, sender) =>
   {
     let subscription = Subscription.fromURL(message.url);
-    if ("title" in message)
-      subscription.title = message.title;
-    if ("homepage" in message)
-      subscription.homepage = message.homepage;
-
     if (message.confirm)
     {
+      if ("title" in message)
+        subscription.title = message.title;
+      if ("homepage" in message)
+        subscription.homepage = message.homepage;
+
       ext.showOptions(() =>
       {
         sendMessage("app", "addSubscription", subscription);
       });
     }
     else
     {
-      subscription.disabled = false;
-      FilterStorage.addSubscription(subscription);
-
-      if (subscription instanceof DownloadableSubscription &&
-          !subscription.lastDownload)
-        Synchronizer.execute(subscription);
+      addSubscription(subscription, message);
     }
   });
 
   port.on("subscriptions.get", (message, sender) =>
   {
     let subscriptions = FilterStorage.subscriptions.filter((s) =>
     {
       if (message.ignoreDisabled && s.disabled)
         return false;
       if (s instanceof DownloadableSubscription && message.downloadable)
@@ skipping 24 matching lines @@
     let subscription = Subscription.fromURL(message.url);
     if (subscription.url in FilterStorage.knownSubscriptions)
     {
       if (subscription.disabled || message.keepInstalled)
         subscription.disabled = !subscription.disabled;
       else
         FilterStorage.removeSubscription(subscription);
     }
     else
     {
-      subscription.disabled = false;
-      subscription.title = message.title;
-      subscription.homepage = message.homepage;
-      FilterStorage.addSubscription(subscription);
-      if (!subscription.lastDownload)
-        Synchronizer.execute(subscription);
+      addSubscription(subscription, message);
     }
   });
 
   port.on("subscriptions.update", (message, sender) =>
   {
     let {subscriptions} = FilterStorage;
     if (message.url)
       subscriptions = [Subscription.fromURL(message.url)];
 
     for (let subscription of subscriptions)
     {
       if (subscription instanceof DownloadableSubscription)
         Synchronizer.execute(subscription, true);
     }
   });
 })(this);