dpacassi / disable-javascript

Adds the ability to disable JavaScript on specific sites.
MIT License
272 stars 33 forks source link

Fix global replacement of noscript #40

Closed goffi-contrib closed 6 years ago

goffi-contrib commented 6 years ago

every occurence of noscript was replaced by div in the whole HTML of every <noscript> HTML element, breaking code/URL or replacing text where noscript is used. This patch fixes it by only replacing the first occurence of noscript, i.e. the tag name.

goffi-contrib commented 6 years ago

Hello, this extension is breaking my code which is using noscript is CSS URLs when javascript is disabled. This is because of the g flag used, which is actually useless if you just want to replace tag name by div. This patch fixes it. Thanks.

dpacassi commented 6 years ago

Thanks for the pull request! I was able to reproduce the bug but wasn't happy to use the replace function in the first place. I've updated the code and your issue will be fixed in the next patch release. Thanks again for pointing out to this!

goffi-contrib commented 6 years ago

Hi @dpacassi yes I agree that it's better to not use replace. I also realised that my patch was incomplete anyway as it was not replacing the closing </noscript> tag. Looking forward for next patch release, thanks for this extension :)

dpacassi commented 6 years ago

I've just published version 2.3.1, it might take up to one day for your browsers to update the extension. Let me know if you experience any issues after the update!

goffi-contrib commented 6 years ago

@dpacassi just checked, issue is solved, all good, thanks for reactivity.