Bug: Following link that has return false

Hello,

I’m new to this forum and I’m in need of advice with some Javascript.

I have a xhtml page here:

http://www.bibo.ro/cannedprunes/recipes.html

In Firefox, the latest version, when you click on the link of a recipe it should open a box with the recipe (make it display as it initially has display:none). Instead it opens it but also follows the link (performs the default action) in spite of the fact that my function returns false. It works in Chrome and IE 7& 8 though.

Here’s the code:


window.onload=prepareLinks;
function prepareLinks() {
var recipeLinks=document.getElementsByTagName("a");
for (i=0;i<recipeLinks.length;i++) {
	if(recipeLinks[i].className=="recipe") {
		recipeLinks[i].onclick=function (){
			var recipeId=this.nextSibling.id;
			setVisible(recipeId);
			return false;
			}
		}
	}
	}
function setVisible(obj) {	
	obj = document.getElementById(obj);
	obj.style.display = (obj.style.display == 'block') ? 'none' : 'block';
	obj.parentElement.style.zIndex=(obj.parentElement.style.zIndex != '100') ? '100' : '10';
}

However if I replace the call to setVisible with an alert statement it works just fine, the alert pops up and it doesn’t follow the link.

I also tried returning the call to setVisible and put return false in the setVisible function but doesn’t work either.

I’ll be forever thankful if anyone could point my mistake, I tried everything that crossed my mind and searched for hours on the Internet.

Thank you,
Ciprian

About the only major problem that I can see is that this.nextSibling.id may not give you the sibling that you expect.

With your above code: the this keyword should give you the element that was clicked on, but the nextSibling property can depend on whitespacing and the web browser being used.

I see on your test page though that your code is different from that in your post.

If you’re having trouble gaining access to the link that triggered the event, you can use the event information to get the appropriate target:


function eventHandler(evt) {
    evt = evt || window.event;
    var targ = evt.target || evt.srcElement;
    if (targ.nodeName === 3) {
        targ = targ.parentNode;
    }
    // You can now do stuff with the target element
    // ...
}

I don’t think parentElement works in Firefox… parentNode should work better.

Thank you for your replies.

pmw57: this.nextSibling works just fine. I tested it with an alert statement before and it gives me the correct id to pass to the setVisible function (I removed all white space and cleaned the document anyway).

CletusSpuckler: the setVisible function works also. It does do what it’s required. The only problem is that the link is still followed.

I also tried using addEventListener and event.preventDefault() and it will work in Chrome but not Mozilla.

This is just crazy, it makes no sense (to me it doesn’t at least).

Also, I’m trying different stuff, so the live code may differ from what I pasted here.

Ciprian

Nope, setVisible does not work in Firefox. The first two lines of code work, so the div will be displayed, but obj.parentElement.style does not work and that’s what causes the error…

Okay, let’s find out why.

Here’s some of the code that you’re using:


function prepareLinks() {
var recipeLinks=document.getElementsByTagName("a");
for (i=0;i<recipeLinks.length;i++) {
	if(recipeLinks[i].className=="recipe") {
		this.addEventListener("click",function (e){

When the prepareLinks function runs, the this keyword is going to be the window. You don’t want to attach the event listener to the window. You want to attach it to the link instead.

this.addEventListener is wrong, and needs to be changed.

What do you think it needs to be changed to?

Thanks a bunch, it did work. I removed the

obj.parentElement.style.zIndex=(obj.parentElement.style.zIndex != '100') ? '100' : '10';

and it now works in Mozilla also.

I put that line of code because in IE7 it will display the recipe div under all the other links.

I now have to find another solution to deal with IE7. Any ideas?

pmw57 : That was just me trying. I did manage to get that to work using recipeLinks[i].addEventListener.

However I changed the code to how it was because addEventListener doesn’t work in IE.

I’ll be grateful if you could help me further with the bug in IE7.

Thank you for your time.

I’m not that skilled with CSS, and I don’t have IE7 to hand for testing purposes.

When it comes to presentation issues, the good people in the CSS forum are excellent at helping there.

I found a script for IE detection on the Microsoft site and I now check for IE7 and add that line of code just for IE7 and it works.

Here’s the code for IE version detection in case anybody else will need it:

function getInternetExplorerVersion()
// Returns the version of Internet Explorer or a -1
// (indicating the use of another browser).
{
  var rv = -1; // Return value assumes failure.
  if (navigator.appName == 'Microsoft Internet Explorer')
  {
    var ua = navigator.userAgent;
    var re  = new RegExp("MSIE ([0-9]{1,}[\\.0-9]{0,})");
    if (re.exec(ua) != null)
      rv = parseFloat( RegExp.$1 );
  }
  return rv;
}

Thank you both for your help :slight_smile:

On further research it looks that the problem was cause by parentElement and when I replaced it with parentNode it works just fine without browser detection and special code for IE7.

I must have not refreshed it enough the first time I tried CletusSpuckler’s advice because it didn’t work then. This goes to teach me that one should refresh many times before trying further solutions :slight_smile:

Ciprian

That’s a relief, as it prevents me from raging on about the evils of browser detection.

Haha, I was kinda thinking the same thing :slight_smile:

Yes, I know that’s not a good practice (from my books and from what I read on the web, didn’t hit my head against this problem yet) that’s why I tried further to solve my problem without using it and finally fixed the bug.