Jump to conversation
Unresolved conversations (9)
@annevk annevk Jan 10, 2020
I worry a little bit about this as we are also writing tests that assume that images outside the viewport are not immediately fetched and the normative requirement does not allow for what this note says is allowed. We probably need to adjust the normative requirements a bit to allow for this, but that will make testing for not fetching complicated.
Outdated
source
@domfarolino
@Jessidhia Jessidhia Sep 17, 2019
Does/should the lazy loading state interact with the fetch priority? One would think intersecting, but not yet in-viewport images, should have less priority than intersecting _and_ in-viewport images.
Outdated
source
@domfarolino
@chrishtr chrishtr Aug 1, 2019
I think the spec needs to explicitly call out relation to the load event. In particular: if a lazy image happens to be on the screen, presumably it delays the load event until the image is loaded. If it's offscreen, it doesn't? When is the final call made about this question? I suggest the following definition: After the first time rendering is updated after the HTML source is finished parsing, observe which images are to-be-loaded and which are not, based on layout at that time. Delay the load event until the to-be-loaded images load. This also means that the load event will have to fire after the first render post-HTML parsing, which also needs spec text.
Outdated
source
@zcorpan @chrishtr @hsivonen
@annevk annevk Jul 9, 2019
So this means navigation ends up being scheduled at a random point in the future. This seems problematic as the eventual navigation will end up copying state from the document causing the navigation. E.g., if you change your base URL a bunch of time it depends on when the user scrolls what base URL your `srcdoc` document will end up with. I don't think that's what we want to enshrine here. Is this tested?
Outdated
source
@domfarolino @annevk
@jakearchibald jakearchibald Apr 17, 2019
What if this data arrives later than the full image, and the two results have different dimensions? It feels like the ordering here needs to be a 'must' not a 'may'.
Outdated
source
@jakearchibald @othermaciej
@yoavweiss yoavweiss Mar 28, 2019
"CSS boxes of those images intersect" feels a bit handwavy. You probably want to link from that to relevant definitions
Outdated
source
@yoavweiss yoavweiss Mar 8, 2019
When is this algorithm called? Should it somehow be hooked into "update the image data"? Are we sure that it is being called after the image element was added to the DOM? (so that we'd know if it has a `<picture>` parent)
Outdated
source
@yoavweiss
@jakearchibald jakearchibald Jul 25, 2018
What happens if this value is changed after element creation? Eg, if I switch from "on" to "off". Feels like the image should immediately load. Worth spelling out?
Outdated
source
@zcorpan @domenic
@jakearchibald jakearchibald Jul 25, 2018
I think there are cases where we need to define 'auto'. Expectations are pretty strong when it comes to `new Image()`. Perhaps "off" is the default unless the image was created by the regular document parser (not `innerHTML` and `createContextualFragment`).
Outdated
source
@zcorpan @domenic
Resolved conversations (55)
@annevk annevk Jan 10, 2020
```suggestion ```
Outdated
source
@annevk annevk Dec 11, 2019
It would be nicer I think to restructure this a bit so we periodically schedule a task to check (or do this as part of "update the rendering") as the current setup with invoking this from in parallel and then that algorithm having a note saying not to do that is rather weird and self-contradictory.
Outdated
source
@domfarolino @annevk
@annevk annevk Dec 11, 2019
```suggestion otherwise.</p></li> ```
source
@annevk annevk Dec 11, 2019
```suggestion <li><p>Let <var>delay load event</var> be true if the <code>img</code>'s <span>lazy loading ```
source
@annevk annevk Dec 11, 2019
```suggestion ```
Outdated
source
@annevk annevk Dec 11, 2019
```suggestion ```
Outdated
source
@zcorpan zcorpan Nov 22, 2019
Maybe say "early" or "when layout is known" or so. In my mind it still loaded lazily.
Outdated
source
@domfarolino domfarolino Nov 21, 2019
Maybe we should find a more general location for the definition of "intersect the viewport", but I don't have strong opinions on where this should go, so I'll defer to others.
Outdated
source
@annevk @domfarolino @domenic
@domfarolino domfarolino Nov 21, 2019
I don't fully know the reasoning for providing _both_ forms of width/height (via CSS & HTML attrs), so I mostly ripped this from your comment @annevk let me know if it's OK
Outdated
source
@domfarolino @annevk @Malvoz
@annevk annevk Nov 21, 2019
Should we add an example, here or elsewhere?
Outdated
source
@domfarolino @annevk
@annevk annevk Nov 21, 2019
Indentation here is slightly wrong. It needs to be aligned with the `<li>`.
Outdated
source
@annevk annevk Nov 19, 2019
This algorithm talks about an observer, what's the observer here? Perhaps they should add some high-level primitive we can use here? Also, since `<p>` is the only child of the `<li>`, there's no need for newlines before and after. Same for the next item.
Outdated
source
@domfarolino @annevk @domenic
@smaug---- smaug---- Nov 11, 2019
I would drop 'auto' for now. That is a major interoperability concern. Just have lazyload to default to eager for now and then let pages to use lazy if wanted.
Outdated
source
@annevk annevk Nov 4, 2019
Please remove this newline and one newline between `</dt>` and `<dd>` as well as the one before `</dl>`.
Outdated
source
@annevk annevk Nov 1, 2019
I thought one of the major features of lazy loading was to not delay the main load event (aka body/Window load event)?
Outdated
source
@domfarolino @zcorpan @annevk
@annevk annevk Nov 1, 2019
It would be easier to read if this returned a boolean.
Outdated
source
@domfarolino
@annevk annevk Nov 1, 2019
This does not actually explain why that needs to be done.
Outdated
source
@domfarolino @annevk
@zcorpan zcorpan Oct 29, 2019
```suggestion influence their behavior differently, sites might be able to more-easily identify specific user ``` Avoid RFC 2119 keyword here.
Outdated
source
@domfarolino
@zcorpan zcorpan Oct 29, 2019
Calling the "compute the intersection" algorithm here seems wrong. It means that the next paragraph only applies for images that intersect according to this requirement. The point here is to differentiate between "obtain images immediately or on demand", which is now more like "follow the processing model for obtaining images immediately or follow the processing model for obtaining images on demand". That is, the "on demand" business is when you disable all images, but still want to be able to load some images by right clicking them and select "Load this image". (The spec only allows such a mode when scripting is disabled.)
Outdated
source
@domfarolino
@domfarolino domfarolino Oct 29, 2019
The "Auto" state indicates the UA should choose which state out of "Eager" or "Lazy" to resolve to, so here I'm just calling it out explicitly, which I think is OK.
Outdated
source
@domfarolino @zcorpan
@annevk annevk Oct 29, 2019
Why was this removed?
Outdated
source
@domfarolino
@Jessidhia Jessidhia Sep 17, 2019
Why is there a requirement to enqueue a task to call this algorithm, if the first thing the algorithm does is enqueue a continuation task?
Outdated
source
@domfarolino
@Jessidhia Jessidhia Sep 17, 2019
This seems to assume that the `auto` behavior is / will always be the same as `eager`, but the check a few lines before seems to assume `auto` is like `lazy`. Perhaps it's better to wait for the attribute to _change_ from its current setting?
Outdated
source
@domfarolino
@zcorpan zcorpan Sep 12, 2019
This seems like it would eagerly load all disconnected images, which doesn't seem desired (see https://github.com/whatwg/html/pull/3752#issuecomment-484177815 ).
Outdated
source
@domfarolino
@smaug---- smaug---- Jul 26, 2019
FWIW, I think we shouldn't have auto state at all, at least not if its behavior isn't clearly defined and not left to UA to decide. And it is super risky to have it as the default behavior (suddenly by default UAs can start to load iframes and images etc later).
Outdated
source
@annevk annevk Jul 9, 2019
I don't understand how going in parallel here is compatible with the processing model when this attribute is not present.
Outdated
source
@domfarolino @annevk
@annevk annevk Jul 9, 2019
I don't understand how this works if we're in parallel.
Outdated
source
@domfarolino @annevk
@annevk annevk Jul 9, 2019
It's unclear from the description how this vector is more unique than the `User-Agent` header. If they're equivalent, that's probably worth noting.
Outdated
source
@domfarolino @annevk
@annevk annevk Jul 9, 2019
user agents should not*
Outdated
source
@agdillon @domfarolino
@annevk annevk Jul 9, 2019
defer fetching*
Outdated
source
@domfarolino
@annevk annevk Jul 9, 2019
This needs rewording, an element does not load.
Outdated
source
@domfarolino
@annevk annevk Jul 1, 2019
This isn't referenced, are we no longer doing range requests? Perhaps someone can point me to an up-to-date description of this feature?
Outdated
source
@domfarolino
@jakearchibald jakearchibald Apr 17, 2019
Needs updating?
Outdated
source
@JoshTumath JoshTumath Apr 13, 2019
To save needing to fetch image metadata, could we first check if the `width` and `height` attributes are specified and assume the image dimensions from that?
Outdated
source
@bengreenstein @mikesherov
@JoshTumath JoshTumath Apr 13, 2019
Typo 🙂 ```suggestion user agent determines is reasonable to retrieve metadata.</p></li> ```
Outdated
source
@rajendrant
@domenic domenic Apr 1, 2019
This isn't a dfn; it should be a link to another spec's definition. See https://github.com/whatwg/wattsi/blob/master/Syntax.md#cross-specification-cross-references
Outdated
source
@zcorpan zcorpan Mar 6, 2019
Don't use may here
Outdated
source
@zcorpan @domfarolino @ahmadawais
@zcorpan zcorpan Mar 6, 2019
Don't use must here
Outdated
source
@domfarolino @zcorpan @ahmadawais
@domenic domenic Dec 20, 2018
These steps should only run "If the full image is returned by this fetch". Probably also you need a guard like "If the resource type and data corresponds to a supported image format, as described below" (see step 24 of updating the image data").
Outdated
source
@domenic domenic Dec 20, 2018
You want to remove "it should be used and not refected" since that's not clear.
Outdated
source
@domenic @bengreenstein
@domenic domenic Dec 20, 2018
What if the element _does_ use srcset or picture? What are selected source and selected pixel density set to? In the next step you parse selected source, so your code will "crash" in that case currently.
Outdated
source
@domenic domenic Dec 20, 2018
Just noticed: you should wrap "fetch image metadata" in a `<dfn>`, so that it's easy to link to.
Outdated
source
@domenic domenic Dec 18, 2018
"should be rejected" is not very precise. Maybe "must not be used as metadata"?
Outdated
source
@domenic domenic Dec 18, 2018
"it should be used and not refetched" needs to be formalized, in terms of the "list of available images". In particular it seems like you'd want a nested series of steps that creates a cache key and stores it in the list. Cf. steps 6.2 and other usages of "key" in the "update the image data" algorithm.
Outdated
source
@domenic @bengreenstein
@domenic domenic Dec 18, 2018
This should just be element's node document's relevant settings object; see https://whatpr.org/html/3752/images.html#updating-the-image-data step 19.
Outdated
source
@domenic domenic Dec 18, 2018
urlString is not a defined variable at this point. Probably you need to copy a step similar to https://whatpr.org/html/3752/images.html#updating-the-image-data steps 4 and 6.1. (Although, you can use "return" instead of "abort this inner set of steps".) However, this raises an interesting and tricky question: how do you handle this for images with srcset, or images inside a picture? Maybe we need more extensive surgery :(. E.g. maybe we need to factor out more of that algorithm. Curious how this maps to Blink's implementation.
Outdated
source
@domenic domenic Dec 18, 2018
"the CSS boxes those" -> "the CSS boxes of those"
Outdated
source
@jakearchibald jakearchibald Jul 25, 2018
Should we be explicit that this mustn't happen if the primary fetch for the resource has begun?
Outdated
source
@domenic @jakearchibald
@jakearchibald jakearchibald Jul 25, 2018
I'm not sure how much hand-waving is ok here, but this seems pretty hand-wavey. We plan to use the metadata to lay out the image, right? Is that worth documenting as a possible consequence? ~~What happens if the response is a 404 or 500? Feels like we could call the image a failure at that point. We should probably do that for all not-ok responses, unless it looks like a consequence of making the range request (416).~~ What happens if the response is a 200? We'd download the whole image, would we download it again? Perhaps we'd add it to the image memory cache in that case? Or maybe terminate the fetch after fetching the metadata bytes? What happens if the response is a 206, but we fail to parse the start? Again, we could call the image a failure, and not bother fetching any more. What happens if the response is a 206, but the range starts at a point other than 0. It's pretty important to reject the response in this case, as reading it as if it were the start of the resource could leak data. What happens if the response is a 206, starts at 0, but returns fewer bytes than requested?
Outdated
source
@domfarolino @domenic @bengreenstein
@annevk annevk Jul 25, 2018
I don't think this works for `about:blank` and such, does it? What does this map to in say Blink's C++?
Outdated
source
@domenic @scott-little
@annevk annevk Jul 25, 2018
@jakearchibald could you please review this?
Outdated
source
@domfarolino
@annevk annevk Jul 25, 2018
Shouldn't this be kept and something equivalent be added for lazyLoad?
Outdated
source
@domenic @annevk
@domenic domenic Jun 27, 2018
You'll want to explicitly refer to and use https://fetch.spec.whatwg.org/#concept-request-add-range-header here
Outdated
source
@domenic @bengreenstein
@domenic domenic Jun 27, 2018
The first sentence here is good, but the others are not. This is not the image request, so it should not be associated as such. Instead, I'd say something like "Fetch request, and use the metadata contained in response as appropriate for the image."
Outdated
source
@bengreenstein
@domenic domenic Jun 27, 2018
No need for the `&#x231B; ` parts here and elsewhere.
Outdated
source
@bengreenstein