开发者

strange npe in jQuery method

I have this jQuery method which works fine if I am on a page where there are elements found which have class .singlePaneOfGlassBlock.

function replaceRightClickIcefacesMethod() {
        //I only want this to run on IE for now, but the content 
        //is prepared to run on other browsers too
        if (jQuery.browser.msie) {
            var elem = jQuery(".singlePaneOfGlassBlock"), oldName = elem
                    .attr("oncontextmenu"), fn = String;

            //IE returns function instead of string so let's try to take the string
            //in the right way
            if (typeof oldName == "function") {
                oldName = elem[0].getAttributeNode("oncontextmenu").value,
                        fn = Function;
            }
            //do the replace
            oldName = oldName.replace('Ice.Menu.contextMenuPopup',
     开发者_开发问答               'contextMenuPopupUpdated');
            //change oncontextmenu with the new value
            jQuery.each(elem, function() {
                this.setAttribute("oncontextmenu", fn(oldName));
            })
        }
    }

But it failes with this error:

`'undefined' is null or not an object`

on the line where it tries to do the replace if I am on other pages where those type of elements are missing...

I have add a checking to see if it's null or if it contains that string before doing the replace, but it still fails:

if (jQuery(oldName).length || oldName.indexOf("Ice.Menu.contextMenuPopup") != -1) {
        oldName = oldName.replace('Ice.Menu.contextMenuPopup',
                'contextMenuPopupUpdated');
        jQuery.each(elem, function() {
            this.setAttribute("oncontextmenu", fn(oldName));
        })
    }

Can you give me a solution?

Thanks.

UPDATE: when I'm asking a solution I'm just asking on how to correct this method to not run when that type of elements are not found?


Can you give me a solution?

I'm not sure - for starters please format your code so that it's clearer what it's supposed to do. It looks like you may have a missing pair of curly brackets after the if condition.

At the very least, don't use comma-separated expressions to achieve two assignments - put that block in curly brackets and split out the two assignments properly. Then the code logic should become clearer.

In any event, since your problem only occurs when that class doesn't exist on the page, that should be your first test:

function replaceRightClickIcefacesMethod() {
    if (jQuery.browser.msie) {
        var elem = jQuery(".singlePaneOfGlassBlock");

        if (elem.length > 0) {   // new test here!

            var oldName = elem.attr("oncontextmenu");
            var fn = String;

            if (typeof oldName == "function") {
                oldName = elem[0].getAttributeNode("oncontextmenu").value;
                fn = Function;
            }

            //do the replace
            oldName = oldName.replace('Ice.Menu.contextMenuPopup',
                                       'contextMenuPopupUpdated');

            //change oncontextmenu with the new value
            jQuery.each(elem, function() {
                this.setAttribute("oncontextmenu", fn(oldName));
            });
        }
    }
}


Update: Re your updated code in the question:

function replaceRightClickIcefacesMethod() {
    if (jQuery.browser.msie) {
        var elem = jQuery(".singlePaneOfGlassBlock"), oldName = elem
                .attr("oncontextmenu"), fn = String;

        if (typeof oldName == "function") {
            oldName = elem[0].getAttributeNode("oncontextmenu").value,
                    fn = Function;
        }

        // ====> This is where the bug is, you're assuming you have
        //       a string at this point, but you don't necessarily,
        //       because if there is no match at all, `attr` will
        //       return `undefined` or `null` (I forget which).
        //do the replace
        oldName = oldName.replace('Ice.Menu.contextMenuPopup',
                'contextMenuPopupUpdated');
        //change oncontextmenu with the new value
        jQuery.each(elem, function() {
            this.setAttribute("oncontextmenu", fn(oldName));
        })
    }
}

to fix:

function replaceRightClickIcefacesMethod() {
    if (jQuery.browser.msie) {
        var elem = jQuery(".singlePaneOfGlassBlock"), oldName = elem
                .attr("oncontextmenu"), fn = String;

        if (typeof oldName == "function") {
            oldName = elem[0].getAttributeNode("oncontextmenu").value,
                    fn = Function;
        }

        // ====> Add this `if`:
        if (oldName) {
            //do the replace
            oldName = oldName.replace('Ice.Menu.contextMenuPopup',
                    'contextMenuPopupUpdated');
            //change oncontextmenu with the new value
            jQuery.each(elem, function() {
                this.setAttribute("oncontextmenu", fn(oldName));
            });
        }
    }
}

Original answer:

It seems to me you're missing some braces (at least):

function replaceRightClickIcefacesMethod() {
if (jQuery.browser.msie) {
    var elem = jQuery(".singlePaneOfGlassBlock"), oldName = elem
            .attr("oncontextmenu"), fn = String;
    if (typeof oldName == "function") { // <=== Added { here
        oldName = elem[0].getAttributeNode("oncontextmenu").value,
                fn = Function;

        oldName = oldName.replace('Ice.Menu.contextMenuPopup',
                'contextMenuPopupUpdated');
        jQuery.each(elem, function() {
            this.setAttribute("oncontextmenu", fn(oldName));
        })
    } // <=== Added } here
}
}

Here's the above with the indentation corrected:

function replaceRightClickIcefacesMethod() {
    if (jQuery.browser.msie) {
        var elem = jQuery(".singlePaneOfGlassBlock"), oldName = elem
                .attr("oncontextmenu"), fn = String;
        if (typeof oldName == "function") { // <=== Added { here
            oldName = elem[0].getAttributeNode("oncontextmenu").value,
                    fn = Function;

            oldName = oldName.replace('Ice.Menu.contextMenuPopup',
                    'contextMenuPopupUpdated');
            jQuery.each(elem, function() {
                this.setAttribute("oncontextmenu", fn(oldName));
            })
        } // <=== Added } here
    }
}

Here's your original with the indentation corrected:

function replaceRightClickIcefacesMethod() {
    if (jQuery.browser.msie) {
        var elem = jQuery(".singlePaneOfGlassBlock"), oldName = elem
                .attr("oncontextmenu"), fn = String;
        if (typeof oldName == "function")
            oldName = elem[0].getAttributeNode("oncontextmenu").value,
                    fn = Function;

        // Note how these statements are NOT protected by the `if`
        // above, and so if `oldName` is `undefined` (as opposed to
        // being a String or Function.
        oldName = oldName.replace('Ice.Menu.contextMenuPopup',
                'contextMenuPopupUpdated');
        jQuery.each(elem, function() {
            this.setAttribute("oncontextmenu", fn(oldName));
        })
    }
}

Or it may be that you're missing an else clause and probably a further defensive if:

function replaceRightClickIcefacesMethod() {
    if (jQuery.browser.msie) {
        var elem = jQuery(".singlePaneOfGlassBlock"), oldName = elem
                .attr("oncontextmenu"), fn = String;
        if (typeof oldName == "function") {
            oldName = elem[0].getAttributeNode("oncontextmenu").value,
                    fn = Function;
        }
        else {    
            oldName = oldName.replace('Ice.Menu.contextMenuPopup',
                    'contextMenuPopupUpdated');
        }
        if (oldName) {
            jQuery.each(elem, function() {
                this.setAttribute("oncontextmenu", fn(oldName));
            });
        }
    }
}

But it's really hard to say, I can't quite follow the logic. The function path part of it doesn't seem to change anything, but the string path part does...


Instead of jQuery(oldName).length, why don't you just do oldName != null?

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜