XF 1.5 Why function($, window, document, _undefined)?

[Nerbert]

I'm working on a project that involves using a custom JavaScript popup function that assigns action functions to any of several buttons defined in the arguments of the popup function. I want to be able to fire either ordinary functions or methods depending on the context. I declare the context to be either window or this in the arguments. This worked both ways in a test in an admin CP project (where popup() was a method) but isn't working for window in a forum project. In both cases all the code was inside the usual anonymous function I see in all XenForo JavaScript. Here's part of the code I'm using:

$popup.one("click", ":button",  $.proxy(function(e) {
    action = $(e.target).data("action");
    context = typeof context != 'undefined' ? context : this ;
    if(typeof context[action] == "function"){
        fade = context[action](e);
    }
    if(fade !== false) {
        $overlay.fadeOut(1000, function(e) {
            $overlay.remove();
            if(typeof overlayResize != "undefined") $(window).off("resize", overlayResize);
        });
    }
}, this));

As I said above, context is set as one of the of popup function arguments and actions are assigned to the buttons in an object

{buttonValue1 : action1,  buttonValue2 : action2, .... }

The problem I'm having is I can't use an ordinary function definition function myFunction(e) { ...} but have to use window.myFunction = function(e) { ....}. I suspect this has to do with the anonymous function that assigns window as an argument. So what's this anonymous function for anyway? And is there a clean way to fix this?

Spoiler: The whole function

function openPopup($parent, formId, html, buttons, width, top, context, event) {
    var $overlay, $popup, buttonString = "", value, type, action, $form, fade = true, overlayResize, left;
    $("#overlay").remove();
    if(typeof overlayResize != "undefined") $(window).off("resize", overlayResize);
    $parent = typeof $parent == "string" ? $($parent) : $parent ;
    width = typeof width == "number" ? width + "px" : "200px" ;
    top = typeof top == "number" ? top + "px" : "200px" ;
    if(typeof buttons != "udefined" && Object.keys(buttons).length > 0) {
        $.each(buttons, function(buttonValue, action) {
            type = action == "submit" || action == "reset" ? action : "button" ;
            action = action == "submit" || action == "reset" ? "" : action ;
            value = typeof xenPhrase == "undefined" || typeof xenPhrase[buttonValue] == "undefined" ? buttonValue : xenPhrase[buttonValue] ;
            buttonString += '<input type="'+type+'" class="button" value="'+value+'" data-action="'+action+'" />';
        });
    }
    $popup = $(
        '<div id="popup" style="position:fixed;top:'+top+';">'+
            '<form id="'+formId+'" class="xenForm formOverlay" style="width:'+width+';">'+
                '<div>'+
                    html +
                    '<div class="buttons">'+buttonString+'</div>'+
                '</div>'+
            '</form>'+
        '</div>'
    );
    $overlay = $('<div id="overlay" class="xenOverlay" style="height: ' + $parent.innerHeight() + 'px; margin-bottom: -' + $parent.innerHeight() + 'px;"></div>');
    $overlay.prependTo($parent).append($popup).fadeIn(1000);
    $form = $popup.find("form");
    $popup.css("left", ($(document).innerWidth() - 17 - $popup.outerWidth())/2 + "px");
    $(window).on("resize", overlayResize = function() {
        $overlay.css({"height": $parent.innerHeight()+"px", "marginBottom": "-" + $parent.innerHeight() + "px"});
        $popup.css("left", (($(document).innerWidth() - 17 - $popup.outerWidth())/2) + "px")
    });
    if(buttonString != "") {
        $popup.one("click", ":button",  $.proxy(function(e) {
            action = $(e.target).data("action");
            context = typeof context != 'undefined' ? context : this ;
            if(typeof context[action] == "function"){
                fade = context[action](e);
            }
            if(fade !== false) {
                $overlay.fadeOut(1000, function(e) {
                    $overlay.remove();
                    if(typeof overlayResize != "undefined") $(window).off("resize", overlayResize);
                });
            }
        }, this));
        $form.on("submit", function() {
            $overlay.fadeOut(1000, function(e) {
                $overlay.remove();
                if(typeof overlayResize != "undefined") $(window).off("resize", overlayResize);
            });
        });
    }
}

 

[Mike]

Function scoping changes within a closure (or other function). Closures like this are generally specifically to prevent (or limit) changes to the global scope.

I'm not seeing where you're calling openPopup, but if it's in a situation where it explicitly needs to be part of the window object, then you would either need to move it outside the closure or do the window.x-style assignment (which would work in any situation, since window is the global context).

 

[Nerbert]

Is there any difference between forum JS and admin CP Js? The admin CP version of this in my other project worked with every possible combination of methods and functions.

Would closure still work with an anonymous function without the window argument? Or without any arguments at all?

 

[Nerbert]

This works for all combinations of function and method

$popup.one("click", ":button",  $.proxy(function(e) {
    action = $(e.target).data("action");
    if(context === window || typeof context == "undefined") {
        eval("fade = " + action + "(e)")
    }
    context = typeof context != 'undefined' ? context : this ;
    if(typeof context[action] == "function"){ ///////console.info("is func")
        fade = context[action](e);
    }
    if(fade !== false) {
        $overlay.fadeOut(1000, function(e) {
            $overlay.remove();
            if(typeof overlayResize != "undefined") $(window).off("resize", overlayResize);
        });
    }
}, this));

NOTE: The code above has been edited

Hate using eval() though

 

[Mike]

s there any difference between forum JS and admin CP Js?

It's literally the same JS file being loaded, so there shouldn't be.

Would closure still work with an anonymous function without the window argument? Or without any arguments at all?

Technically it would, but we haven't really established that this is the definitive cause; there are potential scoping differences with regards to nested function definitions though. But given that this is presumably custom code being loaded from a separate file or outside the core XF JS, that closure wouldn't be involved. I was guessing that you were adding a closure here yourself. If your code isn't built to function that way, I just wouldn't add a closure.

 

[Nerbert]

I try to make my code mostly like XF's so I prefer to use the closure. I have it working with the eval() option now. I've always read you should avoid eval but I guess the point of closure is to avoid problems with stuff like that. I'm using strict mode and no errors are showing. Thanks

 

[Mike]

I'm using strict mode and no errors are showing.

See the "block level functions" part of this page: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions The way you're defining functions is changed by how strict mode works.

Your best bet is really to adjust your code to not rely on the global scope. Define your functions in local scope so you don't pollute the global namespace and then adjust your click handler to look in a different place for the actions it invokes.

 

[Nerbert]

Your best bet is really to adjust your code to not rely on the global scope. Define your functions in local scope so you don't pollute the global namespace and then adjust your click handler to look in a different place for the actions it invokes.

Well, ordinary functions are in global scope and people use them all the time and I want to be able to use them as well, defined the usual way. I want this openPoup function to be completely general and portable even outside XF

If you're looking at overlayResize in the whole function in the first post I'm doing that the safe way, strict mode or not

A safer way to define functions conditionally is to assign a function expression to a variable:

I tested this thoroughly to be sure I wasn't accumulating window resize listeners. When $overlay is removed all its listeners are also removed and the function defined therein is also removed from global scope, so the var overlayResize is undefined again.