New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 774722 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 774718



Sign in to add a comment

Updating the image data should not be called when srcset or sizes attribute are set to the same value

Project Member Reported by hirosh...@chromium.org, Oct 13 2017

Issue description

Spec:
"Updating the image data" must be called when:
https://html.spec.whatwg.org/multipage/images.html#relevant-mutations
> The element's src, srcset, width, or sizes attributes are set, changed, or removed.

Current behavior:
HTMLImageElement::ParseAttribute() calls SelectSourceURL() and ImageLoader::UpdateFromElement() even when srcset or sizes attribute are set to the same value.
Basically this causes a request in Blink but the request hits MemoryCache, so network requests are not made, but image's load event is fired. Thus the code like

img.onload = function() {
  img.srcset = "...";
};

consumes 100% CPU because it repeatedly causes MemoryCache hits and onload() calls.

Firefox doesn't fire img's load event when srcset is set to the same value.

 
Blocking: 774718
Owner: hirosh...@chromium.org
Status: Assigned (was: Untriaged)
Draft CLs:
https://chromium-review.googlesource.com/c/chromium/src/+/719767
https://chromium-review.googlesource.com/c/chromium/src/+/720080

Comment 3 by y...@yoav.ws, Oct 31 2017

Cc: y...@yoav.ws
Thanks for working on this! I guess that the main part here that might be tricky is potential web compat issues with changing that behavior (because maybe people rely on the load event in these cases). Could you create a web-platform-test that checks that and see how other browsers behave here? If everyone other than Chromium agree that onload shouldn't fire, it could be a strong indication that this change is safe.

Otherwise, why have you split `srcset` and `sizes` to separate CLs? And can we also treat `src` while we're at it?

Comment 4 by f...@opera.com, Oct 31 2017

FWIW, I don't agree with the proposed change (the behavior of not dispatching 'load' could be reasonable though, so it'd be good to know what Gecko is in fact doing in these cases.) For the attributes that can contain URLs, a change in to the base URL could affect what the resolved URL is, and hence setting the same textual value could change the actual resolved value. (For 'src' it's called out that setting the same value resets any animation, but the term "changed" can be interpreted in more or less strict ways.)

Sign in to add a comment