Issue 5701 - Inconsistent forum links and DNT link

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 138 matching lines @@
   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")
-      return Utils.getDocLink(message.link);
+    {
+      let {application} = info;
+      if (info.platform == "chromium" && application != "opera")
+        application = "chrome";
+      else if (info.platform == "gecko")
+        application = "firefox";
+
+      return Utils.getDocLink(message.link.replace("{browser}", application));
+    }
 
     if (message.what == "localeInfo")
     {
       let bidiDir;
       if ("chromeRegistry" in Utils)
       {
         let isRtl = Utils.chromeRegistry.isLocaleRTL("adblockplus");
         bidiDir = isRtl ? "rtl" : "ltr";
       }
       else
         bidiDir = Utils.readingDirection;
 
       return {locale: Utils.appLocale, bidiDir};
     }
 
     if (message.what == "features")
     {
       return {
         devToolsPanel: info.platform == "chromium"
-      };
-    }
-
-    if (message.what == "browserInfo")

[Sebastian Noack, 2017/09/25 17:09:22]

Just an idea; how about merging the logic into the app.what[doclink] message:

  if (message.what == "doclink")
  {
    let application = info.application;
    if (info.platform == "chromium" && application != "opera")
      application = "chrome";
    else if (info.platform == "gecko")
      application = "firefox;
    return Utils.getDocLink(message.link.replace("{application}", application);
  }

That way we wouldn't have to introduce a new message sub-type, and the logic
would be reusable.

[saroyanm, 2017/09/25 18:07:53]

Don't have strong opinion, on one hand it makes this specific code reusable, but
on other hand less flexible.
I think having platform + application information in future will help us make
more browser specific tweaks, like show browser specific messages. So we
probably will introduce "browserInfo" if the issue is sending 1 request instead
of 2.

[Sebastian Noack, 2017/09/25 19:07:47]

Also note that the reason you introduced "browserInfo" was to avoid an
unnecessary round-trip to the background page. If you handle this logic in
"doclink", you would avoid yet another round-trip. As for flexibility, I don't
see why it matters much whether to implement it in the background page or on the
content side, you are in control of both, and can just adapt the logic (or move
it back to the content) whenever this becomes necessary. However, handling it in
the "doclink" message seems to yield the simplest solution for now.

[saroyanm, 2017/09/25 20:50:12]

I agree with you, looks much more better now.
Done.

-    {
-      return {
-        platform: info.platform,
-        application: info.application
       };
     }
 
     return info[message.what];
   });
 
   port.on("app.listen", (message, sender) =>
   {
     getListenerFilters(sender.page).app = message.filter;
   });
@@ skipping 236 matching lines @@
     if (message.url)
       subscriptions = [Subscription.fromURL(message.url)];
 
     for (let subscription of subscriptions)
     {
       if (subscription instanceof DownloadableSubscription)
         Synchronizer.execute(subscription, true);
     }
   });
 })(this);