Issue 6684 - Issue Reporter Active Tab Screenshot

popup.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 85 matching lines @@
   {
     window.clearTimeout(activateClickHide.timeout);
     activateClickHide.timeout = null;
   }
   document.body.classList.remove("clickhide-active");
   browser.tabs.sendMessage(tab.id, {type: "composer.content.finished"});
 }
 
 function reportIssue()
 {
-  // capture the current visible tab
-  browser.tabs.captureVisibleTab(

[Sebastian Noack, 2018/05/22 15:28:59]

I suppose we also have to add a new permission for that (in metadata.chrome)?
Also did you verify that adding that permission won't cause the extension being
disabled for existing users (some permissions would)?

[a.giammarchi, 2018/05/22 15:55:31]

AFAIK there's no need for extra permission, tabs would do (tested in b oth
chrome and FF)

[Sebastian Noack, 2018/05/22 16:01:10]

Oh cool, never mind then.

-    null,
-    {format: "png"},
-    (screenshot) =>
-    {
-      // setup a listener to provide once tab url
-      // and base64 screenshot data once asked
-      browser.runtime.onMessage.addListener(
-        function onMessage(event)
-        {
-          if (event.type === "issue-reporter")
-          {
-            browser.runtime.onMessage.removeListener(onMessage);
-            return Promise.resolve({
-              url: tab.url,
-              screenshot

[Sebastian Noack, 2018/05/22 15:28:59]

Nit: It appears that this literal would fit on one line without wrapping.

[a.giammarchi, 2018/05/22 16:14:09]

Done.

-            });
-          }
-        }
-      );
-
-      // create an active/selected issue reporter tab
-      browser.tabs.create({
-        selected: true,

[Sebastian Noack, 2018/05/22 15:28:59]

The "selected" property is deprecated, use "active" instead.

[a.giammarchi, 2018/05/22 15:55:31]

Should I use both to be sure it works? I think I used selected after trying in
older Chrome/FF, but I'm not fully sure anymore.

Thanks for hints.

[Sebastian Noack, 2018/05/22 16:01:10]

That is not necessary, "active" was introduced long before the oldest version of
any browser we support.

[a.giammarchi, 2018/05/22 16:14:08]

Done.

-        url: browser.runtime.getURL("/issue-reporter.html?" + tab.id)
-      });
-    }
-  );
+  // the issue reporter needs to take a screenshot of the
+  // currently active tab, which is why active is explicitly
+  // set as false, in case option defaults will ever change.
+  browser.tabs.create({
+    active: false,
+    url: browser.runtime.getURL("/issue-reporter.html?" + tab.id)
+  }).then(() =>
+  {
+    window.close();
+  });
 }
 
 function toggleCollapse(event)
 {
   let collapser = event.currentTarget;
   let collapsible = document.getElementById(collapser.dataset.collapsible);
   collapsible.classList.toggle("collapsed");
   togglePref(collapser.dataset.option);
 }
 
@@ skipping 210 matching lines @@
       {
         if (event.target.id == "notification-optout")
           setPref("notifications_ignoredcategories", true);
 
         notificationElement.hidden = true;
         browser.runtime.sendMessage({type: "notifications.clicked"});
       }
     }, true);
   });
 });