0

In my view, I am showing an image:

<img src="https://some-host/img1.jpg" onerror="showNoPhotoIcon(this);">

I have defined the function showNoPhotoIcon in a separate file called common.js

/*========== when image is not available ==========*/
function showNoPhotoIcon(image) {
    image.onerror = "";
    image.src = '/images/no-photo-icon.jpg'
    return true;
}

I am referencing common.js from my view, the following line is at the very bottom of my view (html page)

<script src="/Scripts/app/common.js"></script>

But the function is not accessible from the view, when the image is missing, the code tries to call showNoPhotoIcon function and I get the following error:

Uncaught ReferenceError: showNoPhotoIcon is not defined

Update1:

After I get the error, I tried to confirm the function is defined or not => it is.

enter image description here

Update2:

This seems to be a timing issue, because sometimes it works and sometimes it does not.

Hooman Bahreini
  • 14,480
  • 11
  • 70
  • 137
  • 2
    your code is probably trying to call `showNoPhotoIcon` in code ABOVE where you're loading common.js – Jaromanda X Sep 24 '19 at 22:37
  • Use an event listener instead? Rather than putting the js inside an attribute? – evolutionxbox Sep 24 '19 at 22:38
  • @JaromandaX: Yes, I believe so... but I have been told that it is better to add all the css files on top of html and .js files at bottom... – Hooman Bahreini Sep 24 '19 at 22:38
  • @evolutionxbox - an event listener may not necessarily be added in time to handle the error event – Jaromanda X Sep 24 '19 at 22:41
  • @JaromandaX aw poo... what if it gets created during a page event, like dom ready? – evolutionxbox Sep 24 '19 at 22:42
  • @HoomanBahreini - I tried dummying up the code with the handler at the bottom of the page, and it still handled the error - are you sure `showNoPhotoIcon` is in the global context? Go to the browser console, and type `console.log('showNoPhotoIcon')` – Jaromanda X Sep 24 '19 at 22:43
  • @evolutionxbox - it surprised me! – Jaromanda X Sep 24 '19 at 22:47
  • OK, so it's defined in global context - my "dummy" code was obviously flawed – Jaromanda X Sep 24 '19 at 22:48
  • @JaromandaX: Thanks, but the console.log('showPhotoIcon') says: undefined? Is it possible that I am losing binding because I am updating the image using ajax? – Hooman Bahreini Sep 24 '19 at 22:51
  • no, the console.log I told you to do was incorrect `console.log(showNoPhotoIcon)` - without the quotes ... or just type `showNoPhotoIcon` and enter - but I did further testing and it doesn't matter, your code will result in the error you've shown – Jaromanda X Sep 24 '19 at 22:52

3 Answers3

1

I've found having the handler code loaded in an external JS at the bottom of the body element makes the error handling inconsistent - but that's where you want JS code, which I agree with, so ...

One way to make the error handling consistent is as follows

First, don't use onerror attribute

<img src="https://some-host/img1.jpg" alt="" />

Secondly, change your showNoPhotoIcon as follows, because it will be called with an error event as the argument - this makes removeEventListener easy

function showNoPhotoIcon(e) {
    const image = e.target;
    image.removeEventListener('error', showNoPhotoIcon);
    image.src = '/images/no-photo-icon.jpg'
}

Add this code in the common.js

document.querySelectorAll('img').forEach(img => {
    if (img.naturalWidth === 0) {
        img.addEventListener('error', showNoPhotoIcon);
        img.src = img.src;
    }
});

This checks if the image width is 0, if so, it "reloads" it after attaching an error handler

It's a bit hacky in my view, you could also just assume if the width is 0 that the image doesn't exist, but, networks being networks, you never know - at least this way I think you are guaranteed to find and "fix" all broken images

Jaromanda X
  • 53,868
  • 5
  • 73
  • 87
  • "I've found having the handler code loaded in an external JS at the bottom of the body element makes the error handling inconsistent".. thanks for this, it explains the problem... are you able to share any link that you found about this please? – Hooman Bahreini Sep 24 '19 at 23:14
  • @HoomanBahreiniq - no, because it's just an observation based on my experience, nothing I've researched :p – Jaromanda X Sep 24 '19 at 23:16
  • Thanks again, I found a comment on [this answer](https://stackoverflow.com/questions/92720/jquery-javascript-to-replace-broken-images/92819#92819) by Prestaul which says the same thing about on error handler. – Hooman Bahreini Sep 24 '19 at 23:31
  • @HoomanBahreini - yeah, too bad that question and answers were jquery based :D – Jaromanda X Sep 24 '19 at 23:41
  • Personally I think using inline js for handling error is a better solution, its shorter and easier to understand. I appreciate your answer, because it points out the cause of the error. – Hooman Bahreini Sep 24 '19 at 23:48
0

The accepted answer explains the issue which is the error handler not being loaded when error happens.

In my case I changed the handler to use inline js:

<img src="https://some-host/img1.jpg" onError="this.onerror=null; this.src='/images/no-photo-icon.jpg';"
Hooman Bahreini
  • 14,480
  • 11
  • 70
  • 137
-1

Your code looks fine, maybe is the url in your script tag. Try to run your function in the same html page to double check. I don't see any reason to set image.onerror = ""; and return true;

the best way should be to apply an event listener to the image, or images, so you don't have to write the function in the html tag.

document.querySelectorAll('img').forEach(image =>{
     image.addEventListener('error', (e) =>{
          // load the placeholder image
     });
});
  • a) `onerror` is not a correct event for addEventListener, and b) that won't work consistently since the error event could have fired before the javascript is even loaded – Jaromanda X Sep 24 '19 at 23:04