1

My project is a Chrome extension that will do the following.

  • Push the extension icon.
  • Popup will appear (from popup.html)
  • 5 buttons will be in the popup.
  • When you click one of the four buttons, one javascript code will be executed.
  • close popup window.

So depending on the answer of this post over here

Detect a button click in the browser_action form of a Google Chrome Extension

(big ups to Michael for his enormous help)

This example is only for one button. Created it with only one of my javascript code and works perfect. But when it comes to put all of the 5 buttons i 've tried to make this kind of coding but it didnt work at all (im new at javascript code so dont hate)

Here are the codes

MANIFEST.JSON

{
   "background": {
      "scripts": [ "background.js" ]
   },
   "browser_action": {
      "default_icon": "img/icon.png",
      "default_title": "TITLE",
      "default_popup": "popup.html"
   },
   "icons": {
      "128": "img/icon_128.png",
      "19": "img/icon19.png",
      "38": "img/icon38.png",
      "48": "img/icon_48_2.png"
   },
   "manifest_version": 2,
   "name": " NAME",
   "description": " DESCR ",
   "permissions": [ "activeTab" ],
   "version": "2.0"
}

POPUP.HTML

<html>
    <head>
        <script src="popup.js"></script>
        <style type="text/css" media="screen">
            body { min-width:250px; text-align: center; }
            #click-me-l { font-size: 20px; }
            #click-me-f { font-size: 20px; }

        </style>
    </head>
    <body>
        <button id='click-me-l'>Click1</button>
        <button id='click-me-f'>Click2</button>

    </body>
</html>

POPUP.JS

    function clickHandler(e) {
        chrome.extension.sendMessage({directive: "popup-click-l"}, function(response) {
            this.close(); // close the popup when the background finishes processing request
        });
    }


 document.addEventListener('DOMContentLoaded', function () {
        document.getElementById('click-me-l').addEventListener('click', clickHandler);

    })


     function clickHandler(e) {
        chrome.extension.sendMessage({directive: "popup-click-f"}, function(response) {
            this.close(); // close the popup when the background finishes processing request
        });
    }


document.addEventListener('DOMContentLoaded', function () {
        document.getElementById('click-me-f').addEventListener('click', clickHandler);

    })

BACKGROUND.JS

chrome.extension.onMessage.addListener(
    function(request, sender, sendResponse) {
        switch (request.directive) {

             case 1 "popup-click-l":
            // execute the content script
            chrome.tabs.executeScript(null, { // defaults to the current tab
                file: "script1.js", // script to inject into page and run in sandbox
                allFrames: true // This injects script into iframes in the page and doesn't work before 4.0.266.0.
            });

        case 2 "popup-click-f":
            // execute the content script
            chrome.tabs.executeScript(null, { // defaults to the current tab
                file: "script2.js", // script to inject into page and run in sandbox
                allFrames: true // This injects script into iframes in the page and doesn't work before 4.0.266.0.
            });
            sendResponse({}); // sending back empty response to sender
            break;
        default:
            // helps debug when request directive doesn't match
            alert("Unmatched request of '" + request + "' from script to background.js from " + sender);
        }
    }
);

So the codes in the link are working PERFECT for only 1 button. in this example i am trying to make it work for 2 buttons but i cant find what im doing wrong. If anyone has any idea i would appreciate it.

Thanks a lot for your time!!!

(UPDATE 2. Updated codes for 2 buttons but not working.)

Community
  • 1
  • 1
  • Warning: you edited your question in a way that invalidates an existing answer. In general, you want to avoid that. – Xan Mar 11 '15 at 13:59
  • @Xan i said to Teepeemm that i will edit it... i didnt do it on purpose so the answer will be off topic. Im just starting from the very begging of my question. –  Mar 11 '15 at 14:11
  • _You probably mean "beginning"_ It's not off-topic, it's just a risky edit if you make an answer seem incorrect while it was. If you cleared it with the author of the answer then there's no problem. – Xan Mar 11 '15 at 14:13
  • yes it was a typo. anyway his codes were a bit confusing to me cause non of them seem to work for my problem. –  Mar 11 '15 at 14:28
  • Since your goal is five buttons, my recommendation would be to go back to the original version that was trying for two buttons. Getting one button to work and then asking "how do I get more" doesn't seem to be as interesting of a question. – Teepeemm Mar 11 '15 at 15:28
  • @Teepeemm ok i edited it again for 2 buttons. could you please upload somewhere your codes? :) –  Mar 13 '15 at 11:05
  • @SteliosM. Please read on how [switch-case statement](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/switch) works. `case 1 "something":` is just a syntax error. – Xan Mar 13 '15 at 13:01
  • @Xan if you look at the link with the 1 button example, its the same without the (1 or 2) after the "case" but anyway both codes dont work. –  Mar 13 '15 at 17:54

1 Answers1

2

You’re defining clickHandler twice, so only the second one counts. One fix would be:

function clickHandler(e) {
    chrome.extension.sendMessage({"directive": e.target.id}, function(response) {
        this.close(); // close the popup when the background finishes processing request
    });
}

In general, you’re repeating yourself too much. You could combine your DOMContentLoaded events into one:

document.addEventListener('DOMContentLoaded', function () {
    document.getElementById('click-me-l').addEventListener('click', clickHandler);
    document.getElementById('click-me-f').addEventListener('click', clickHandler);
})

but even better would be to put all the buttons into an array, so that popup.js is now:

function clickHandler(e) {
    chrome.extension.sendMessage({"directive": e.target.id}, function(response) {
        this.close(); // close the popup when the background finishes processing request
    });
}

document.addEventListener('DOMContentLoaded', function () {
    var buttons = document.getElementsByTagName("button");
    for ( var i = 0 ; i < buttons.length ; i++ ) {
        buttons[i].addEventListener('click',clickHandler);
    }
})

(And I’d recommend button { font-size: 20px; } in your style instead of five separate ids.)

Finally, your switch statement is buggy. Once you start a case, you’ll keep going until you get to a break, so that case "popup-click-l" hits both cases. You could have a separate executeScript for each case, but even better would be to assign to fileName based on the case, and have a single injection at the end. Or best of all would be to have a javascript object define which files go with which ids, so that background.js is now:

chrome.extension.onMessage.addListener(
    function(request, sender, sendResponse) {
        var injected = {
            "click-me-l": "script1.js",
            "click-me-f": "script2.js"
        };
        chrome.tabs.executeScript(null, {
            "file": injected[request.directive],
            "allFrames": true
        });
        sendResponse({});
    }
);

Fundamentally, this comes back to a point I made in a comment: browser extensions are a bad way to learn javascript, because you’re learning two separate things at the same time. Your difficulties with switch, {}, and generally following the code is a javascript problem. Not seeing when the console tells you about syntax errors is more of a browser extension problem. And your biggest problem is that you’re not seeing which error is which.

Teepeemm
  • 4,331
  • 5
  • 35
  • 58
  • I suppose an ugly way to fix things would be to rename one of your `clickHandler` functions. But you shouldn't need five separate function definitions that all do the same thing. – Teepeemm Mar 10 '15 at 16:54
  • Thanks for your response, but your answer didnt help me a lot. Its true that i have 2 times the clickHandler and i checked out your 1st code for this. But still didnt even work for only 1 button. guess that something else is wrong on the "background.js" but also your code didnt make the 2 buttons work. But why the "case" is a problem?? in my logic i say "in case of "popup-click-l" do this, in case of "popup-click-f" do this".... why its a problem here and it hits all the buttons??? –  Mar 10 '15 at 18:25
  • I'm not sure what would be wrong with the events (javascript is also not my strong suit). Your case logic is wrong because it says "in case of "popup-click-l" do this and then do "popup-click-f" stuff. When you do one case, you keep doing cases until you hit a `break`. – Teepeemm Mar 10 '15 at 18:48
  • aww... i understand... anyway i think im missing something... in the first code that you write the "directive" why is in quotes?? –  Mar 10 '15 at 18:55
  • In an object literal (those `{...}` things), if the keys (left hand sides) are pure strings (like `directive`, `file`, and `allFrames`), then the quotes are optional. I've gone to putting them in quotes in my code because (1) they're strings, not variables, and (2) they're not optional in json – Teepeemm Mar 11 '15 at 00:49
  • ok... but really i cant understand what should i change... because i replaced the parts that you mentioned, as you mentioned and it didnt work... i understand something wrong?? (i dont say that you didnt explain me anything but i cant figure out how each button can call one different script.) –  Mar 11 '15 at 11:12
  • i will edit the first post so there will be no missunerstandings –  Mar 11 '15 at 13:40
  • 1
    I found my two typos: (1) `addEventListenter` was misspelled, (2) the keys for `injected`' now agree with the id's. The extension now works for me. But if you're having trouble following the code, you may need to find a javascript tutorial. Browser extensions are a bad way to learn javascript, because you're also having to learn the extension api at the same time. – Teepeemm Mar 11 '15 at 15:26
  • Since your extension works, could you please upload the code of the files in github or anywhere else to check them out and so if anybody else is trying to find my question to see the codes??? Thanks a lot for your time man i really appreciate it!!! –  Mar 12 '15 at 09:22
  • 1
    @SteliosM. I've explicitly stated what goes in the javascript files. Your manifest and html will work as is. I'd prefer to roll back to your original post, though, as your recent edits have made it inconsistent with my answer (and even with itself, e.g., `clickHandler2`). – Teepeemm Mar 13 '15 at 19:43
  • But im telling you, i've copied your codes and it doesnt execute the scripts. –  Mar 13 '15 at 20:21
  • It turns out you need to add the permissions `"http://*/*", "https://*/*"` (or whatever website you're aiming for). Yet another example of the difficulty of learning javascript and browser extensions at the same time. Don't forget to debug your scripts; there's several different consoles that you may need to locate: `background.js`, `popup.js`, and the page you're modifying. – Teepeemm Mar 13 '15 at 20:53
  • but since the example with one button works without permission or debug, why should this one not work? :P –  Mar 13 '15 at 21:23
  • The example you link to has the permissions. If you're talking about the second version you posted with only a single button, my guess based on how the rest of this answer has gone is that it wasn't actually working. – Teepeemm Mar 13 '15 at 23:33
  • of course its working. i dont have any benefit if i lie to you. anyway in your suggestion is there any other way to write the background.js??? –  Mar 14 '15 at 00:26
  • I don't think you're lying. It's that your question and the now two dozen comments lead me to believe that you aren't proficient enough with javascript and browser extensions to correctly diagnose what is and isn't working. There are of course other ways to write `background.js`, but what I've answered is one of the better ways. Anyways, I think that I've made my answer as good as I can and don't know that anyone will gain by me working on it. If you're still having trouble understanding it, I would recommend working through a javascript tutorial; they're a dime a dozen. – Teepeemm Mar 14 '15 at 13:47