CODEWITHSUNDEEP
javascriptgoogle-chrome-extension
RenderedNonsense
RenderedNonsense

Reputation: 136

Cross origin issue with Chrome extension when chrome.tabs.executeScript(...)

I have a userScript that I run on a company website to remove certain information from the page (based on JavaScript regex replace).

I have decided to make the userScript into a Chrome extension and I would like to add options for running different flavors of the script.

My first objective is to make the script run when the user presses one of the options in the popup.html.

Currently, my chrome.tabs.executeScript() call attempts to access the document and replace its bodyHTML with its bodyHTML minus replaced regex.

However, the call to the script's meat function normalExecution() appears to be blocked by cross origin constraints?

I see the following errors:

Uncaught ReferenceError: normalExecution is not defined

Uncaught DOMException: Blocked a frame with origin "https://website.com" from accessing a cross-origin frame

Below is my popup.html and popup.js code.

In summary, the HTML code has 3 divs which act as buttons. Currently, I have them all set to have a click-action-handler that should execute the code in normalExecution(), but I get the errors above.

Popup.js:

function click(e) {
  chrome.tabs.executeScript(null, {code:"normalExecution();"}, null);
  window.close();
}

var normalExecution = function(){
    console.log('normal execution engaged');
    var allSpans = document.getElementsByTagName("span");
    for(var i = 0; i < allSpans.length; i++) {
        try{
            if (allSpans[i].className.indexOf("textblock") > -1) {
                //check if the ">" is part of switch output, if not then procede
                if(allSpans[i].innerHTML.regexIndexOf(/sw.&gt;.*/g) < 0){
                    //console.log('true');
                    allSpans[i].innerHTML = allSpans[i].innerHTML.replace(/&gt;.*/g, '    ---- Removed ----    ');
                    allSpans[i].innerHTML = allSpans[i].innerHTML.replace(/(-----).*/g, '    ---- Removed Dashes (beta) ----    ');
                }
                else{
                    console.log('switch CLI output detected');
                }
            }
        }catch(e){}
    }
};


document.addEventListener('DOMContentLoaded', function () {
  console.log('adding event listener'); 
  var divs = document.querySelectorAll('div');
  for(var i = 0; i < divs.length; i++){
    divs[i].addEventListener('click', click);  
  }
});


String.prototype.regexIndexOf = function(regex, startpos) {
    var indexOf = this.substring(startpos || 0).search(regex);
    return (indexOf >= 0) ? (indexOf + (startpos || 0)) : indexOf;
};

popup.html [body only]:

<body>
<div id="Disabled">Disabled</div>
<div id="Normal">Normal</div>
<div id="Super-power">Super-power</div>

Upvotes: 0

Views: 1051

Answers (2)

Makyen
Makyen

Reputation: 33296

You are currently trying to execute a function in a content script code which you define in your popup.js. Thus, it is not defined in the content script context/scope. As a result, you get a ReferenceError when you try to execute it in the content script context/scope using chrome.tabs.executeScript()

There is no reason to have normalExecution defined within your popup.js. You are not using it there and appear to have no intent to use it there. Much better would be to move this content script out into a separate file, and inject that file:

popup.js:

function click(e) {
  chrome.tabs.executeScript({file:"/contentScript.js"});
  window.close();
}

document.addEventListener('DOMContentLoaded', function () {
  console.log('adding event listener'); 
  var divs = document.querySelectorAll('div');
  for(var i = 0; i < divs.length; i++){
    divs[i].addEventListener('click', click);  
  }
});

contentScript.js:

(function() {
    //Don't change the prototype of a built in type just to use it _once_.
    //  Changing the prototype of a built in type is, generally, not a good idea. Sometimes,
    //  it is the right thing to do, but there are potential issues. Without learning what
    //  those issues are, it is better to avoid changing the prototype, particularly if
    //  you are only using it once.
    //String.prototype.regexIndexOf = function(regex, startpos) {
    function regexIndexOf(str, regex, startpos) {
        var indexOf = str.substring(startpos || 0).search(regex);
        return (indexOf >= 0) ? (indexOf + (startpos || 0)) : indexOf;
    };

    console.log('normal execution engaged');
    var allSpans = document.getElementsByTagName("span");
    for(var i = 0; i < allSpans.length; i++) {
        try {
            if (allSpans[i].className.indexOf("textblock") > -1) {
                //check if the ">" is part of switch output, if not then proceed
                if(regexIndexOf(allSpans[i].innerHTML,/sw.&gt;.*/g) < 0){
                    //console.log('true');
                    allSpans[i].innerHTML = allSpans[i].innerHTML.replace(/&gt;.*/g
                        , '    ---- Removed ----    ');
                    allSpans[i].innerHTML = allSpans[i].innerHTML.replace(/(-----).*/g
                        , '    ---- Removed Dashes (beta) ----    ');
                } else {
                    console.log('switch CLI output detected');
                }
            }
        } catch(e) {}
    }
})();

popup.html:

<body>
<div id="Disabled">Disabled</div>
<div id="Normal">Normal</div>
<div id="Super-power">Super-power</div>

Using .replace() on innerHTML is usually a bad idea

Using .replace() on the innerHTML property and assigning it back to innerHTML has multiple potential negative effects. These include both potentially disrupting the HTML code by changing values contained within the actual HTML elements, instead of in text; and breaking already existing JavaScript by removing event handlers which are listening on descendant elements. You should only apply such changes to text nodes. If you are in control of the HTML which you are changing it might be reasonable to perform the .replace() on the innerHTML property. if you are not in control of all HTML and JavaScript used on the page (and in other extensions), then there is a significant chance that you will run into problems.

My answer to: Replacing a lot of text in browser's addon shows one way to perform the .replace() only on text nodes. The following additional examples of only changing text nodes are more complex in some ways and less complex in other ways than needed in this case. However, one shows limiting the text nodes to only those contained within a particular tag (in the example, <p> tags). In each of the following, the text node is replaced with a <span> which includes additional HTML (which does not appear needed here): Replace each word in webpage's paragraphs with a button containing that text, Change matching words in a webpage's text to buttons, and Highlight a word of text on the page using .replace(). The examples less complex than necessary in that you are wanting to affect the content of <span> elements. As a result, you will need to account for situations where you have multiple <spans> some of which are, potentially, descendants of others. None of the above examples handles that case as all of them deal with the situation where potentially selecting descendants is not possible (either all text on the page, or text in <p> elements).

Additional inefficiencies in your content script

There are additional things in your content script (normalExecution() and regexIndexOf()) which could be done more efficiently. Addressed below are:

  • You are currently using regexIndexOf() to just test for the non-match of a RegExp within a string. You are not currently using the added capability of regexIndexOf() to provide a starting index. For the functionality you are using, there are multiple already existing methods: String.prototype.search(), RegExp.prototype.test(), and String.prototype.match().

  • You are using document.getElementsByTagName("span"); and then detecting the presence of "textblock" in the className. You can do this is one step by using querySelectorAll():

    document.querySelectorAll("span[class*=textblock]");

    However, the more likely situation is that you are actually attempting to detect to see if the <span> has a class which is exactly textblock. In that case, you would want to use: 

    document.querySelectorAll("span.textblock");

contentScript.js:

(function() {
    console.log('normal execution engaged');
    //You may actually want the selector "span.textblock", which matches spans with the
    //  class "textblock" (e.g. <span class="foo textblock someOtherClass">).  However, the
    //   following matches your current code which checks to see that the className contains
    //  "textblock" somewhere (e.g. <span class="footextblockbar someOtherClass">).
    var allSpans = document.querySelectorAll("span[class*=textblock]");
    for(var i = 0; i < allSpans.length; i++) {
        try {
            //check if the ">" is part of switch output, if not then proceed
            //This does not account for the possibility of having the &gt both after 
            //  a 'sw.' and by itself.
            //It is unclear if you are really wanting /sw.&gt/ or /sw\.&gt/ here (i.e. is 
            //  the "." intended to match any character, or an actual "."). Which
            //  is really desired will depend on the text you are trying to not change.
            if(allSpans[i].innerHTML.search(/sw.&gt;.*/g) < 0 ){
                //console.log('true');
                allSpans[i].innerHTML = allSpans[i].innerHTML.replace(/&gt;.*/g
                    , '    ---- Removed ----    ');
                allSpans[i].innerHTML = allSpans[i].innerHTML.replace(/(-----).*/g
                    , '    ---- Removed Dashes (beta) ----    ');
            } else {
                console.log('switch CLI output detected');
            }
        } catch(e) {}
    }
})();

Upvotes: 1

jlblca9l_kpblca
jlblca9l_kpblca

Reputation: 504

I don't think it's about cross origin constraints. When you execute "normalExecution();" it actually injects this string to the page and executes it at the page's context. Since normalExecution is not defined at the page (instead it is defined at your popup.js), you get ReferenceError. You need to do something like this

chrome.tabs.executeScript(null, {code: "(" + normalExecuton + ")();"}, null);

Upvotes: 0

Related Questions