Snippet of JavaScript code is breaking other JavaScript

Question

I manage a forum that is hosted software so I don't have access to the source code, I can only add JavaScript to the page to achieve what I need done.

I'm trying to replace the first instance of certain text keywords on all pages with hyperlinks. I'm also geo-targeting those links based on country code.

I managed to piece together the code below by reading other answers on this site, and it actually works and does exactly what I want it to do, but it seems to be messing up other JavaScript (or jQuery?) Already loaded on the page previously.

Can anyone see anything wrong with this code that would cause it to affect other JavaScript code?

var c = null;
$.get("https://pro.ip-api.com/json/?key=XXXXXXXXXX", function(data) {
  c = data.countryCode;
  if (c == "GB") {
    var thePage = $(".MessageList");
    thePage.html(thePage.html().replace('KEYWORD1 ', '<a href="http://www.exampleGBlink1.com">KEYWORD1</a> '));
    thePage.html(thePage.html().replace('KEYWORD2 ', '<a href="http://www.exampleGBlink2.com">KEYWORD2</a> '));
  } else {
    var thePage = $(".MessageList");
    thePage.html(thePage.html().replace('KEYWORD1 ', '<a href="http://www.exampleELSElink1.com">KEYWORD1</a> '));
    thePage.html(thePage.html().replace('KEYWORD2 ', '<a href="http://www.exampleELSElink2.com">KEYWORD2</a> '));
  }
  console.log(data.countryCode);
}, "jsonp");

Another problem I have with this code: it's also replacing instances of the keywords in my HTML code, so I added the extra blank space at the end of the keywords as a hack around this.

EDIT:

If I add the code below underneath the code above then everything works properly and my problems are gone, what gives? This file doesn't exist and I get a console error: "Failed to load resource: net::ERR_INSECURE_RESPONSE"

<script src="https://www.RandomDomain.com/test.js"></script>

What is the problem that's causing your JS? Is your console sending you any error messages? — Lixus
– Lixus, Commented Apr 21, 2017 at 15:33
i'm not getting any console errors but other functions of the site are not working properly — BudV
– BudV, Commented Apr 21, 2017 at 15:34

Adam Nierzad · Accepted Answer · 2017-04-21 15:42:08Z

2

Try wrapping your JavaScript in an immediately invoked function expression (IIFE). This will avoid variables you use being registered on the global namespace. It's very likely for a variable name like c to be used by a minified script that is loading on your page.

IIFE:

(function () {
  // Your JavaScript here.
})();

answered Apr 21, 2017 at 15:42

Adam Nierzad

9326 silver badges19 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

BudV Over a year ago

Thank you for your reply, this didn't seem to solve my issue but it's much appreciated

andym1978 · Accepted Answer · 2017-04-21 17:53:54Z

0

One problem I see is with the method you are using to replace the page content. When you extract html elements as a string by using "x.html()", you end up with the html elements in string form, but all of the attached event handlers are not included. When you then update the html elements by calling thePage.html(updatedHtmString), the html elements are restored as expected, but all of the attached event handlers are now lost. This could easily cause issues many js ui frameworks that heavily use event handlers.

Personally, I would traverse the DOM tree using jquery selectors, find the elements having html text content with those keywords, and replacing the keyword with a newly inserted element.

Instead of replacing the entire ".MessageList" DOM tree, you are only changing the text in the elements and adding a new element. No attached events will be lost.

I've added an example FIDDLE and function below. In the fiddle, you can see how your use of .html is removing event handlers. In the function below, I adjusted it to only change the first occurrence by adding ':first' jquery selector.

JSFiddle Demo

// Ugly but functional function to do the replacement.
function GoodReplaceOnlyFirstOccurence(findText, replaceWith) {

    var $targElem = $(".MessageList1");

    $targElem.children().each(function() {
        var $elem = $(this);
        $( ":contains('"+findText+"'):first", $elem ).html(function () {
          return $(this).html().replace(findText, replaceWith); 
        });
    });
}

edited Apr 21, 2017 at 17:53

answered Apr 21, 2017 at 16:51

andym1978

614 bronze badges

9 Comments

Roamer-1888 Over a year ago

This looks to be a likely fix. It might be simpler to stick with the full HTML replacement but ensure it is done before the other js attaches event handlers. (Assuming of course that the keywords are not put in place by the other js).

BudV Over a year ago

Thanks andy this is very helpful, I'm not sure what to do to the code to accomplish what you suggested, can you point me somewhere good to research this?

BudV Over a year ago

Roamer, surprisingly the full html replacement is only breaking one tiny thing on the site so I'm hoping I can keep it simple

BudV Over a year ago

It's also very important that I only replace the first instance of each keyword on the page, I can't replace all instances

andym1978 Over a year ago

I added a demo solution, but it replaces all instances of the keyword. It could probably be adjusted without too much effort to only replace first occurrence.

|

Collectives™ on Stack Overflow

Snippet of JavaScript code is breaking other JavaScript

2 Answers 2

1 Comment

9 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

9 Comments

Your Answer

Sign up or log in

Post as a guest

Related