Open Bug 1572360 Opened 3 years ago Updated 4 months ago

img.decode() regression on macOS

Categories

(Core :: ImageLib, defect, P2)

70 Branch
defect

Tracking

()

Webcompat Priority P3

People

(Reporter: karlcow, Unassigned)

References

(Depends on 1 open bug, )

Details

  1. Go to https://eyen.fr/
  2. Note how images are missing

The images are visible in Safari.

<div class="carousel-item active">
  <img class="" src="/images/placeholder.png" data-src="/images/en/screenshots/screenshot4.png" data-srcset="/images/en/screenshots/screenshot4@2x.png 2x" width="1336" height="948">
</div>

with css

.carousel-item {
	position: relative;
	display: none;
	-webkit-box-align: center;
	-ms-flex-align: center;
	align-items: center;
	width: 100%;
	transition: -webkit-transform 0.6s ease;
	transition: transform 0.6s ease;
	transition: transform 0.6s ease, -webkit-transform 0.6s ease;
	-webkit-backface-visibility: hidden;
	backface-visibility: hidden;
	-webkit-perspective: 1000px;
	perspective: 1000px;
}
.carousel-fade .carousel-item {
	-webkit-transition: opacity .6s ease-in-out;
	transition: opacity .6s ease-in-out;
}
.carousel-item.active, .carousel-item-next, .carousel-item-prev {
	display: block;
}

The console shows the message:

EncodingError: Invalid image request.

Thomas found out that

  1. It works for me on Linux, so odds are that it's Retina-related.
  2. mozregression gives this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1501794

see https://webcompat.com/issues/36948#issuecomment-518046459 for the script involved in decoding the image.

We reject because between asking for the decode() and actually running the microtask to request the decode the src of the img changes from https://eyen.fr/images/en/screenshots/screenshot1.png to https://eyen.fr/images/en/screenshots/screenshot1@2x.png. Per spec we are supposed to reject when this happens. But I haven't determined if we are changing the uri at the right time so it's still possible that it's a gecko bug. This explains why you need a retina screen, because you need to img src to change to the hi res version.

It looks like the image element is created, then we queue a microtask to resolve the src set and load (this changes the uri of the image), then the script calls decode() on the image, then the microtask runs, then the microtask to actually request the decode runs, and by this time the uri has changed as a result of the srcset.

Yeah, this is a Gecko bug. We need to check the request generation when the microtask for the decode runs, not when we enqueue the decode microtask.

Priority: -- → P2

Now I see why it is designed that way. The image loading algorithm from the spec uses a microtask. But for setting img.src Gecko just does it synchronously without a microtask, for srcset and other things it correctly uses a microtask. So the current behaviour was designed to allow img.src = something, img.decode() to work, but that means that img.decode on imgs with srcset might not work.

If we adjust img.decode to only check for mutations to the img in the microtask (instead of when we queue the microtask) that fixes this bug but regresses tests that do img.decode then set img.src and expect the promise to be rejected.

We can probably fix this by tracking which calls are sync and which are microtask in our img element implementation, keeping two generation counters in image loading content, and then checking each generation counter at different points in time depending if the call comes from a sync or microtask.

Webcompat Priority: --- → ?
Webcompat Priority: ? → P3

I think bug 1076583 would fix this.

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