0

This is what I have right now, and it does not work:

<script>
jQuery(function($){
  $(document).ready(function(){
    $("a").attr('onclick', 'return goog_report_conversion(' + this.attr('href') + ')');
  });
});
</script>

Am I using the 'this' keyword incorrectly?

msanford
  • 11,803
  • 11
  • 66
  • 93
cstoddart
  • 39
  • 1
  • 9
  • 3
    Don't do that. You should use actual event handlers. – SLaks Oct 23 '17 at 19:32
  • 2
    No, the line of code is not interpreted yet. So `this` will not refer to the `a` being clicked. (If that was what you wanted it to refer too) – Jimenemex Oct 23 '17 at 19:34

1 Answers1

4

If you're not going to assign the onclick inline in the element, I see no reason why you wouldn't just use a standard event handler. You can read more about the benefits here.

I'd suggest this:

$("a").click(function() {
    var href = $(this).attr("href");
    return goog_report_conversion(href);
});

Also, your first two lines of code are performing the same task, so you won't need both. (More info here)

jQuery(function($){ ... });

//  OR

$(document).ready(function() { ... });

If you're wondering why yours doesn't work:

attr() doesn't have its own this arguments.

(Tracing it upwards, the context in which you're using it would actually refer to the document, as the scope is defined by $(document).ready( ... ) )

If you were to do it that way, you'd likely need to modify your goog_report_conversion() function to accept a parameter of the clicked element, rather than the href. Your this would be included in the string, not concatenated. Using the passed element, you'd fetch the href similarly to how we did in the example above.

function goog_report_conversion(clicked_element) {
    var href = $(clicked_element).attr("href");
    //do something with href
});

$("a").attr("onclick", "return goog_report_conversion(this);");

That being said, please don't do this.

Tyler Roper
  • 21,445
  • 6
  • 33
  • 56
  • 1
    Thank you so much for the detailed answer! I learned a lot from your explanations and from the links you included. – cstoddart Oct 23 '17 at 21:02
  • What if it's not bound to any event. Just change it on load? I just need to override some styles imposed from some other js code. – Lamar Sep 09 '21 at 12:41