Should attribute removal be sync?

[yoavweiss]

Bumped into a Blink bug where attribute removal fails to update width/height synchronously (or at all :/).

Since there are no advantages of making the "update the image data" phase async for this, I suggest we make this phase sync in that case.

@zcorpan - WDYT?

[yoavweiss]

This broke a bunch of tests which made me think it might not be Web compat. Let's further discuss to see if it's a good idea or not.

[zcorpan]

WebKit keeps showing the old image when removing the src attribute.
Blink keeps showing a frame of the same size of the image.
Gecko sync removes the image.
IE11 sync removes the image.
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/3450

When srcset is also present:
WebKit keeps showing the old image until the new one is loaded.
Blink: same
Gecko: same
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/3449

Since the spec has a sync/async split on whether srcset/picture is present, I think it makes sense to do that here also. Given the implemented divergence, it is hopefully not a huge Web compat problem for WebKit/Blink to change to match IE/Gecko.

[zcorpan]

Do we want to abort the pending request sync when src is removed? Looking at the Network tab in devtools, it seems like Gecko does.

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

I think it might be bad to abort requests sync here, since the script might not be done with whatever it's going to do. For instance, consider a script that wants to "upgrade" an <img src> to <img srcset>. If it first removes the src and then sets srcset, this would abort the request. But if it first sets srcset and then removes src, the request is not aborted (assuming the chosen candidate is the same as the pending request).

I think the way to do this in the spec is to add a step right before step 6 (await a stable state):

If the element does not have a srcset attribute specified and it does not have a parent or it has a parent but it is not a picture element, and it does not have a src attribute specified or it does and its value is the empty string, set the current request to the broken state and change the current request's current URL to the empty string.
Note: current request and pending request are not aborted here, and these steps are not aborted.

[yoavweiss]

The first step is probably to try to align that code with Gecko/IE in both Blink and WebKit, and see what breaks and why. I'm looking into that. Will report back.

[Nephyrin]

FWIW the current Gecko behavior is to behave synchronously if: We have no srcset and aren't in a <picture> and there is no queued update the image data task.

https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLImageElement.cpp#896

[Nephyrin]

See also Mozilla bug 1076583

[zcorpan]

@Nephyrin thanks. Can you give an example of there being a queued update the image data task?

[zcorpan]

I think it might be bad to abort requests sync here,

The spec already does that when setting src to a value that is in the "list of available images", and the same reasoning applies there. Possibly we could defer the request abortings until after step 6 in that case also. WDYT?

[Nephyrin]

@Nephyrin thanks. Can you give an example of there being a queued update the image data task?

I believe something like:

<img src="foo" srcset="bar">
<script>
  img.removeAttribute("srcset");
  img.removeAttribute("src");
</script>

Removing src will queue a task, whereas:

<img src="foo" srcset="bar">
<script>
  img.removeAttribute("srcset");
  setTimeout(function() {
    img.removeAttribute("src");
  }, 0);
</script>

Would remove src synchronously, as the task had run and the tag now considered itself "non-responsive". Some of this logic was to prevent changing event ordering when <picture> or <img srcset> were not involved, to be followed up on in aforementioned bug to remove any that old behaviors that are no longer consistent with the spec.