Lazy load script/lib icons

[jerone]

I propose the following solution (see code) to fix the issue with custom userscript icons that are blocking the page.

This fix will lazy load all userscript icons when available by JavaScript (not Node.js), making the page load faster and stops blocking the page load when images aren't returning at all.

Default situation are the standard script/lib icons. Those will only be overridden when the lazy loaded userscript icon returns an useful image.

No functionality is removed or server-side functionality is added.

Example:
Blocking script: https://openuserjs.org/scripts/sizzle/Textarea_Keyboard_Shortcuts
Blocking image: http://sizzlemctwizzle.com/arrrkitty.jpg --> returning 502
Results off the time it takes to load the above blocking image (a clean install since 2 weeks):

Browser:	Result:
Chrome	avg. 5 seconds
IE11	avg. 5 seconds
Firefox	42 seconds (might be an interference with an addon), 5 seconds by @Zren

This will solve:

• Images that take a long time to load because of external server processing.
• Images that take a long time to load because of their size.
• Images that return an error (e.g. 4xx or 5xx).

Other solutions:

• Validate on our back-end upon request. Cons: takes time and extra processing.
• Validate on script upload/update. Cons: images can disappear after they were validated.
• Host them ourselves. Cons: huge storage required.
  All other solutions above can coexists with proposed solution.

This is not related to the 'Mixed Content' issue!

[Zren]

Just going to point out that the Firefox Nightly I use to test with also loads the page in 5s. So it's likely just Jerone getting the 42sec page loads.

Demo: https://oujs.herokuapp.com/scripts/Zren/Textarea_Keyboard_Shortcuts

[jerone]

@Zren commented on 26 nov. 2014 12:01 CET:

Just going to point out that the Firefox Nightly I use to test with also loads the page in 5s. So it's likely just Jerone getting the 42sec page loads.

Added to the list.

[jerone]

Just did a small adjustment to make it even less blocking.

[Martii]

@jerone
Did you make it show up on all views? Yesterday it wasn't showing up on Source Code and Issues (tabs). I can retest shortly today but tomorrow is a holiday so I won't be available to dev test till Sunday as I alluded to earlier.

[jerone]

@Martii commented on 26 nov. 2014 18:37 CET:

@jerone
Did you make it show up on all views? Yesterday it wasn't showing up on Source Code and Issues (tabs). I can retest shortly today but tomorrow is a holiday so I won't be available to dev test till Sunday as I alluded to earlier.

It should, but I will test right now.

Edit:
You're right, will fix it now.

Edit 2;
Fixed.

[Martii]

One other question... Do those only load on the visible viewport? e.g. if someone is using (something like) https://openuserjs.org/scripts/srazzano/Openuserjs_Table_View with 100 entries (or worse all scripts in the list) on the script list page.

[jerone]

@Martii commented on 26 nov. 2014 19:33 CET:

One other question... Do those only load on the visible viewport? e.g. if someones using https://openuserjs.org/scripts/srazzano/Openuserjs_Table_View with 100 entries (or worse all scripts in the list) on the script list page.

For that situation nothing has been changed from what it is now.

[Martii]

Don't you think it would be wise to do that for the benefit of the end-user in the script?

If you need some code reference installWith does this here . You may be able to glean some modified code converted to jQuery to do this in https://github.com/OpenUserJs/OpenUserJS.org/blob/dc9cc68a259103ad4104695de3659f909f6c5b51/views/includes/scripts/lazyIconScript.html .

[jerone]

@Martii commented on 26 nov. 2014 19:48 CET:

Don't you think it would be wise to do that for the benefit of the end-user in the script?

If you need some code reference installWith does this here . You may be able to glean some modified code converted to jQuery to do this in https://github.com/jerone/OpenUserJS.org/blob/lazy-icons/views/includes/scripts/lazyIconScript.html .

Actually @Zren mentioned it already, but I don't want to burn my hands on that part. To make that work it requires constant pooling on the scroll event to know if an image wrapper is into view. With all browsers (desktop, tablet and mobile) supported by this site, I don't want to be responsible for that feature.

This issue only fixes issues with images that are slow or error.

This script has no extra performance penalty on OUJS or the user (it's async). If an user chooses to show 100 entries I'm going to say that that's probably on a desktop with enough bandwidth.

[Martii]

We natively support that QSP used in srazzano's code so this should be a strong consideration (I didn't know that until sizzle told me)...

This will solve:

• Images that take a long time to load because of external server processing.

... Seems to me this falls under this and should be inclusive... if you don't want to do it in this PR open a new issue for it... enhancement label... not team biz because it's something that should happen and back reference this issue please. These event listeners don't impact system performance as much as you are implying and that's assuming that a visitor/user doesn't have JavaScript disabled. Also it's preferable to save even desktop bandwith so you don't get silly letters from your ISP saying "You're an extreme Techie" due to lots of bandwidth used.

[Martii]

... requires constant pooling ...

I did assume you meant "polling" btw which isn't true in event driven models and is why they exist. Now setTimout and setInterval fall into the category you mentioned.

[jerone]

This sounds a little harsh, but I don't care about someone userscript that alters a website (oke, as a scriptwriter I do care, but as a developer, userscripts come second place). Changing websites is a big disadvantage of building userscripts and we all had to deal with that (e.g. Github).

Again, what you're asking is to my opinion a totally other feature (however someone builds it; polling, timers, events, whatever...). The issue here was that images block the browser from finishing. As the test shows, this script will save 5 seconds of loading for every image that is bad.
And even if your feature is build, in the end lazy loading is still required for that feature.

This PR is here and fixes an issue that is happening right now.

[Zren]

Also. Icons should be tiny and take less than a second to download. If it weren't for the fact that we don't validate the URLs at all, we would not consider lazy loading them in the first place. It's still hard for me to accept it's needed but icons are eye candy so it's fine if this breaks them.

The only place that'd benefit from lazy loading is a Script ReadMe/Forum threads that are loaded with screenshots (or big gifs) but the browser probably handles those smartly to begin with anyways. If a page had 20-100 screenshots then lazy loading using the scroll event might be a good idea, but we're don't have that.

[Martii]

As the test shows, this script will save 5 seconds of loading for every image that is bad.

Why not save even more by the viewport check? I do find it unusual that you think desktops don't deserve bandwidth reduction which is inclusive having the browser finish loading.

Anyhow... I'm at -0+ because of this e.g. I'll most likely merge it because we need it even for what you say it's not for (that's why I was on it very quickly when you posted on your branch)... but I think we also need the viewport check... e.g. that will probably end up being a "Just do it" thing. I've asked @sizzlemctwizzle to come in here so maybe he can enlighten us a bit more. Just to note Zren isn't mistake proof (neither am I or anyone else) so don't be completely relying on one devs word no matter how much of a power coder they are... this a lesson I learned from experience.

[jerone]

Why not save even more by the viewport check?

Because it's async and happens on the background, you can't save more time then is saved by this PR.

Anyways, I still believe that the viewport-scroll-enhancement is something different and can be seen as an addition to the lazy loading, but it not required to make the site non-blocking.

This PR is here and fixes an issue that is happening right now.

[Martii]

Because it's async and happens on the background, you can't save more time then that.

Yes you can with the viewport check. If someone bookmarks the QSP'd URI with all scripts (this removes your implication about .user.js being second class scripting... btw that's not the best thing to say to the community of Userscripts) you just made their bandwidth (with timing) much worse. Another reason why this PR is a -0+ (towards the minus for this reason) with me... e.g. we need this PR with some future mitigation.

Anyhow... #449 (comment) should happen and if you don't want to "burn your hands" I'll create the issue for you... but it's not that difficult to create an issue that is highly related to this one and you are welcome to requote with your validating GH scripts.

If it weren't for the fact that we don't validate the URLs at all

This should be briefly mentioned... I've been looking into a SVG scrubber assuming that it is our scope (origin) and not sandboxed in all browsers... other than that, what was said in #399 still stands.... which is another reason why this PR is a -0+ (towards the plus for this reason) with me... e.g. we need this PR.

Other URI's we aren't responsible for period... I occasionally zap an external obfuscated injected script but that's the only instance I can foresee where we should patrol... e.g we're not responsible for some other owned domain under any circumstance especially with images. ... e.g. we need this PR.

As far as eyecandy goes if we didn't need it we wouldn't be in Windows/X11/OSX and we'd all be back in a terminal/DOS prompt (text environment)... ~/o those were the days ~/o ;) :)

[sizzlemctwizzle]

I'm +1 on this PR. Those QSPs were added as a courtesy, not something that we officially guarantee to work perfectly. Additionally, any breakage is purely visual and worth the immediate benefit of merging.

[jerone]

Just for reference and to be correct; lazy loading is not the correct naming for this feature. Other technics like preloading aren’t the right naming also. The technic I used here can be called something like ‘asynchronous broken image detection’ (made it up myself).
Lazy loading is a technic to only load images that are in the users view. This is useful to preserve bandwidth by only loading images that are actually viewed.
Preloading is a technic to load a predefined list of images as soon as possible. This is useful for when new images will be added to the page after some action. A slider for example.

Images are so called 'replace images', making the parser wait for them to return their actual width & height, before re-rendering the page.

The problem with the normal way of adding images is that images, which take a long time to load, will block the entire page from finish rendering. Both previous mentioned technics (lazy load & preload) don’t solve this problem.

The solution to this problem is to download all images when the browser is finished with loading & rendering the page. The JavaScript function this cannot block other actions requested by the user or other features, thus the function needs to run async. Then there needs to be a check to only add the images to the page when the image exists.

How the technic in this PR works:

1. It runs when the page is fully loaded including graphics.
2. It uses setTimeout to run asynchronous.
3. A temporary image holder is added to begin downloading the image, where it keeps checking for the ‘ready’ status.
4. Only when a valid status code (e.g. 200) is returned, the image will be added to the page, replacing the placeholder icon.