Open Bug 1647077 Opened 4 years ago Updated 13 days ago

"lazy loading" implementation on images have a different behaviour depending on the order you define attributes

Categories

(Core :: DOM: Core & HTML, defect, P3)

77 Branch
defect

Tracking

()

Webcompat Priority P3

People

(Reporter: rui.nibau, Unassigned)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

19.19 KB, application/zip
Details
Attached file test.zip

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:77.0) Gecko/20100101 Firefox/77.0

Steps to reproduce:

  • If you define the "loading" attribute at "lazy" AFTER the "src" attribute, it has no effect (all images are loaded, even those not in the viewport).
  • If you define the "loading" attribute at "lazy" BEFORE the "src" attribute, it it applied as expected (only images in the viewport are loaded).

(complete test case as file attachement).

here's the JavaScript used for testing :

    /**
     * Creates a "bad" list of attributes : 'loading' AFTER 'src'
     * @param {Number} index
     * @return {{alt: string, src: string, loading: string}}
     */
    const badObject = (index) => {
        return {alt: `Image ${index}`, src: `${index}.jpg`, loading: 'lazy'};
    };
    /**
     * Creates a "good" list of attributes : 'loading' BEFORE 'src'
     * @param {Number} index 
     * @return {{loading: string, alt: string, src: string}}
     */
    const goodObject = (index) => {
        return {loading: 'lazy', alt: `Image ${index}`, src: `${index}.jpg`,};
    };
    /**
     * Creates an image that will push it's next sibling outside the viewport
     * @param {Object} attrs Object of attributes for the image.
     * @return {HTMLImageElement}
     */
    const createImage = (attrs) => {
        // Creates an image that will push it's next sibling outside the viewport
        const img = document.createElement('img');
        img.style.display = 'block';
        img.style.marginBottom = '120vh';
        // Apply attributes by reading object entries in the defined order.
        for (const [name, value] of Object.entries(attrs)) { img.setAttribute(name, value); }
        return img;
    };
    /**
     * Run tests : insert images into the body and read the `complete` property.
     * Only the first image should have `complete` at true.
     * The test needs 3 lightweight images in the current directory : 1.jpg, 2.jpg, 3.jpg
     * @param {String} title 
     * @param {Function} builder 
     */
    const run = (title, builder) => {
        return new Promise((resolve) => {
            const images = [];
            document.body.innerHTML = '';
            for (let i = 1; i < 4; i += 1) {
                const img = createImage(builder(i));
                document.body.appendChild(img);
                images.push(img);
            }
            console.log(`[TEST] ${title}`);
            window.setTimeout(() => {
                images.forEach((img, index) => {
                    if (index === 0) {
                        console.log(`\t# Image ${img.alt} complete = true`);
                        console.assert(img.complete === true);
                    } else {
                        console.log(`\t# Image ${img.alt} complete = false`);
                        console.assert(img.complete === false);
                    }
                });
                resolve();
            }, 250);
        });
    };

    // Run the tests
    run('With bad objects', badObject).then(() => run('With good objects', goodObject));

Hiro, can you please have a look at this?

Flags: needinfo?(hikezoe.birchill)

I didn't notice it when I fixed bug 1628894. :/

Assignee: nobody → hikezoe.birchill
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(hikezoe.birchill)
See Also: → 1628894

Okay, this is due to bug 1076583. When the src attribute is set to an image, we synchronously start loading the image. But it's not yet clear to me whether we should start the loading asynchronously or synchronously.

Assignee: hikezoe.birchill → nobody
Status: ASSIGNED → NEW
Depends on: 1076583
Severity: -- → S3
Priority: -- → P3
Duplicate of this bug: 1804875

The severity field for this bug is set to S3. However, the following bug duplicate has higher severity:

:edgar, could you consider increasing the severity of this bug to S2?

For more information, please visit auto_nag documentation.

Flags: needinfo?(echen)

:edgar Just to highlight the severity of this issue, I am using a javascript framework (Vue) to template my html, which handles the construction of the DOM. This results in loading="lazy" not having any effect and I don't have any control over this.

^ Even changing the order of the attributes in the template cannot fix this for me.

No longer duplicate of this bug: 1804875
Flags: needinfo?(echen)

Demo: http://software.hixie.ch/utilities/js/live-dom-viewer/saved/11181

Per spec, if the image is in the "list of available images" cache, it should be loaded in step 6 of https://html.spec.whatwg.org/multipage/images.html#updating-the-image-data

If not, step 7 should queue a microtask, which allows script to set attributes in any order and insert the element into a picture element and so forth, before fetching starts.

Then, if the image lazy, step 22 stops to let the lazy loading machinery resume when the image is sufficiently "in view".

As this issue has been open for quite some time, and as the loading argument is now also fully supported by Safari, which makes it a very attractive way to implement lazy loading of pictures, I proposed an addition to MDN to mention this bug, till it is resolved.

Mentioning it here, so that once this gets resolved and closed, we can remove it from MDN again.

Forgot the link to the PR: https://github.com/mdn/content/pull/26778

Webcompat Priority: --- → ?

We don't know about critical breakage at the moment, but breaking lazy loading in some cases might cause issues. One example would be a site using a lot of lazy loaded images, where the perceived performance in Firefox could be significantly worse if we load all images. It would be a higher priority than a P3 if we actually see a webcompat report about this.

Webcompat Priority: ? → P3

This might be a case where we could figure out how often we are seeing the bad order using some kind of static analysis (e.g. HTTP Archive). If we think that lazy loading just isn't working some large percentage of the time that would suggest a higher priority than if the bad case is very rare.

Please consider the impact this bug has on servers too. When Firefox downloads content it wasn't supposed to request, it adds to the cost of keeping servers running and can degrade everyone's experience.

I've found out that it was affecting Piped, a popular privacy-friendly Youtube frontend: https://github.com/TeamPiped/Piped/pull/3264 . Piped instances are run by volunteers and are frequently under heavy load.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: