Why the button area is not working as I expected?

Viewed 76

I am trying to make an auto scroll tool for the browser(Chrome). What i want is a button, so when i click the button, the page should auto scroll down. By the way, I am using an application, whenever i visit a website, the application will inject the script below in the website. Here is my script:

<script>
let btn = document.createElement("button"); 
var isScrolling = false; 
btn.innerHTML = "start scroll"; 
btn.style.cssText = "width:50px;height:50px;float:right;background:grey;position:fixed;right:0;top:50%;opacity:0.5;border-radius:50%"
btn.addEventListener("click", () => onclick); 
let autoScroll = () => {if(isScrolling){window.scrollBy(0, 1);setTimeout(autoScroll, 10)}};
onclick = function() {console.log("click"); isScrolling=!isScrolling; autoScroll()}; 
document.body.appendChild(btn); 
</script>

The problem is when I click the other areas of the page(not the button area), it still will trigger the onclick function. What i want is the page only scroll down when i click the button area, not any other area of the page. Does anyone know where the problem is? You can copy and run the script in the chrome console, any help would appreciate!

3 Answers

I believe you are running into conflicts with your variables and the scripts already on the page. If injecting this script, you can break javascript code already on the page.

To get the variables into their own frame, try this:

<script>
(function () {
    let btn = document.createElement("button"); 
    var isScrolling = false; 
    btn.innerHTML = "start scroll"; 
    btn.style.cssText = 
"width:50px;height:50px;float:right;background:grey;position:fixed;right:0;top:50%;opacity:0.5;border-radius:50%"
    let onclick = function() {console.log("click"); isScrolling=!isScrolling; autoScroll()}; 
    btn.addEventListener("click", onclick); 
    let autoScroll = () => {if(isScrolling){window.scrollBy(0, 1);
    setTimeout(autoScroll, 10)}};
    document.body.appendChild(btn); 
})();

</script>

The problem is that you are storing your onclick function into a global one, because you are not defining it before. This works correctly:

<script>
let btn = document.createElement("button"); 
var isScrolling = false;
const onclick = function() {console.log("click"); isScrolling=!isScrolling; autoScroll()};  
btn.innerHTML = "start scroll"; 
btn.style.cssText = "width:50px;height:50px;float:right;background:grey;position:fixed;right:0;top:50%;opacity:0.5;border-radius:50%"
btn.addEventListener("click", onclick); 
let autoScroll = () => {if(isScrolling){window.scrollBy(0, 1);setTimeout(autoScroll, 10)}};
document.body.appendChild(btn); 
</script>

Pay attention that i'm defining the onclick before using it, and that it is directly used in addEventListener, you shouldn't return it in the arrow function:

// This is WRONG
btn.addEventListener("click", () => onclick);
// This is OK
btn.addEventListener("click", onclick);

The problem is in this line:

btn.addEventListener("click", () => onclick);

You would want either:

btn.addEventListener("click", onclick); // binding a reference for later execution

or

btn.addEventListener("click", () => onclick()); 

However, if you want to later btn.removeEventListener to prevent memory leaks, you would rather want the former variant

It would also be better code if you first declared the onclick handler and then bound it to button, not the other way around :)

Related