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
?
精彩评论