media-condition/mediaquery changes should only affect images in document

[aFarkas]

Currently if a picture/img is removed from DOM, viewport changes are triggering downloads for these elements. See also corresponding chrome bug ticket.

In case of a SPA or AJAX driven website, this yields to a) a lot of memory consumption and b) a lot of network traffic for images, that are removed long time ago.

The spec should be more clear, that if an images is created or a relevant mutation-change happend the resource selection should start (queued) based on the current viewport/media-condition/mediaquery matches, but in case of a mediaquery change on the other hand should only affect images inside the document.

[yoavweiss]

Note that in some scenarios that may not be the desired behavior: e.g. authors may want to remove the element from the DOM only to bring it back into the DOM later one, with the right resource already loaded.

Also, in the current situation, authors can remove the HTMLImageElement from the inside of HTMLPictureElement to make sure viewport changes don't trigger a resource load.

With that said, I'm not against such a spec change, just saying that people may get surprised either way we pick.

[yoavweiss]

Also, seems like the spec has no guidance regarding viewport changes and their impact on triggering resource loading: #157

[pjm4]

FYI - the use case where this is happening is a single page application where the user is moving from a list view to a detail view that displays a responsive image. I remove the detail view after the user returns to the list view. It was at this time that I noticed that the picture element in the view I removed kept firing when I resized the page. My main concern is that the browser was not releasing the memory of the detail views as the user navigated back and forth between the list and detail views.

[zcorpan]

Removing an element from the document (or inserting it to the document) doesn't change the viewport. It's also not a "relevant mutation" (unless you remove/insert img from/into a picture).
https://html.spec.whatwg.org/multipage/embedded-content.html#source-size-2

Moving an element from one document to another is a "relevant mutation" (last bullet point).
https://html.spec.whatwg.org/multipage/embedded-content.html#relevant-mutations

[yoavweiss]

@philipjmurphy The memory concern is probably a viable one, but can probably be addressed by removing the <img> element from its picture parent and removing its srcset attributes. The UA should use that to remove any internal references to these objects that were kept for viewport change notification purposes.

[aFarkas]

To be honest, I don't really understand @zcorpan comment. I know that removing an image or a picture image construct from the document is not a relevant mutation.

However - if an img or picture/img construct is removed from the document and even if there is no reference from JS land to this DOM object or its ancestors - Chrome still references it and still runs the source selection algorithm for it, if the viewport changes. This is obviously bad for network and memory. But it also introduces a totally new concept of how dom nodes have to be removed.

Do you really suggest, that any JS script in the world, which worked element agnostic to change the content of a document has to include now some exceptional JS to treat responsive images differently?

If this issue isn't fixed, every script, which uses the following code:

function replaceHTML(dom, newContent){
    dom.innerHTML = newContent;
}

has to change to something like this:

var cleanImg = function(img){
    img.removeAttribute('srcset');
    img.removeAttribute('sizes');
    if(img.parentNode){
        img.parentNode.removeChild(img);
    }
};

function replaceHTML(dom, newContent){
    Array.prototype.slice.call(dom.querySelector('img[srcset], picture > img')).forEach(cleanImg);
    dom.innerHTML = newContent;
}

I can try to submit a patch for jQuery's $.cleanData method, but I'm pretty sure they will think I'm nuts.

[yoavweiss]

@zcorpan's suggestion on IRC to use weak references to the keep track of nodes for viewport notifications purposes can probably do the trick.
So, this is not a spec issue. Let's continue this on Chrome's bug tracker. Closing.

[zcorpan]

Oh I probably misunderstood what was meant by "viewport changes". I thought you meant that removing an element from the document would associate the img with a different viewport. But I understand now you meant that when the viewport changes its width, it also affects img elements that are not in the document. This is intentional; if you have a reference to the img element, it should continue to work the same as when it's in the document. But if you don't have any references and no event listener for load/error, the browser does not need to keep it around or fetch anything for that element (since it would not be observable other than by looking at network activity).

[zcorpan]

@aFarkas

zcorpan: Actually, I have no time, but this still concerns me. a) I'm not happy with this part: and no event listener for load/error and b) I'm not sure whether the spec should be clearer so that implementers don't overlook this problem

[zcorpan]

The HTML spec sometimes has requirements about garbage collection where it is not obvious, e.g. WebSocket. We can certainly do it here also.

We can also tweak the rules so that it becomes possible to garbage collect imgs without ongoing fetches even if they have load/error listeners, but ideally I think we shouldn't break out-of-document imgs from "working". Maybe an OK tradeoff is to have the relevant mutations still have an effect, but have the environment changes algorithm abort early if the element is not in a document. Thoughts?

[aFarkas]

Correct me if I'm wrong. I thought, that added event listeners aren't taken into account by garbage collectors (only in old IE, if there is a circular reference). Therefore if there is a listener added to the element and there is no (other) reference, the element will be garbage collected.

If it wouldn't be garbage collected, I'm fine with both ways you described above. But if it would be garbage collected I don't see a reason why an event listener should still count for source selection to be run.

[yoavweiss]

On the one hand, maybe the author just took the image out of the document in order to later add it in a different location, expecting it to keep itself up-to-date with the current viewport dimensions?

On the other hand, if we'd rely only on weakrefs and GC in order to avoid downloads here, we will probably end up with a racy experience, where the resources are sometimes downloaded and sometimes aren't. (I'm guessing we do not want to force GC in the middle of JS execution)

So, aborting early for the env change case (as well as weakrefs) probably makes the most sense.

[zcorpan]

Possibly event listeners are not relevant for img, but we need to at least avoid GC while there are outstanding fetches. It should be possible to create an img and not store a reference to preload an image, for instance. And if there are listeners, they should be invoked.

[zcorpan]

I haven't specified anything specific about GC, but technically it's not necessary. I think it's more useful to spend the time writing test cases.