Pass tab.id when possible

miketaylr commented 6 years ago

Please pass an explicit tab.id when possible. Your current code is prone to a race condition, and in the worst case you would leak cross-origin data to other websites (because you are not passing the tabID of the new tab to chrome.tabs.executeScript).

OrionStar25 commented 6 years ago

Hey, I'd like to work on this one!

miketaylr commented 6 years ago

Sounds good, thanks!

OrionStar25 commented 6 years ago

~~I understand the issue, but it would be great if you could please tell me where I could start from. :sweat_smile:~~

function reportIssue(tab, reporterID, tabId) {
  chrome.tabs.captureVisibleTab({ format: "png" }, function(res) {
    let screenshotData = res;
    chrome.tabs.query({ currentWindow: true, active: true }, function(tab, tabId) {
      var newTabUrl = `${PREFIX}${encodeURIComponent(
        tab[0].url
      )}&src=${reporterID}`;
      chrome.tabs.create({ url: newTabUrl }, function(tab) {
        chrome.tabs.executeScript({
          code: `window.postMessage("${screenshotData}", "*")`
        });
      });
    });
  });
}

This is the function which needs modification, but it would be nice if you please elaborate what needs to happen @miketaylr @adamopenweb

miketaylr commented 6 years ago

Check out the following, @OrionStar25:

https://developer.chrome.com/extensions/tabs#method-executeScript https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/executeScript

We need to pass tabId into executeScript.

miketaylr commented 6 years ago

Note that in the latest code, we don't pass in the tab id to reportIssue:

https://github.com/webcompat/webcompat-reporter-extensions/blob/master/shared/base.js#L32

So we need to make sure it ends up there, so it will be in scope for the executeScript call (and that it's the id of the new tab, not the existing tab).

webcompat / webcompat-reporter-extensions

Pass tab.id when possible #85