1286836 - After onload event fires on image old image is still briefly visible

Status: NEW

(Core :: Graphics: ImageLib, defect, P3)

Description

[jt]

Attachment: testcase.html

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36

Steps to reproduce:

Create an Image with a src and load it.
Set its onload event handler to include a style change.
Change the src of the image.


Actual results:

The style change is briefly visible while the old image is shown.


Expected results:

The style change and the new image source showing should happen simultaneously. I don't know if there is supposed to be a guarantee that this happens, but it does in previous versions of firefox and other browsers.

This can be seen in the attached test case when the square sometimes appears to move the first time it changes colour. In version 45 that never happens but in 47+ it does.

This occurs in real life in my Google Maps overlay. When the map stops moving the overlay is supposed to re-centre itself and load a new image at the same time, but in recent versions of firefox it flickers slightly.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

So in the spec there are for example the following (depending on the whether to use existing image or loading something new)

4. Let current request be a new image request whose image data is that of the entry and whose state is set to the completely available state.
5. Update the presentation of the image appropriately.
6. Let the current request's current pixel density be selected pixel density.
7. Queue a task to restart the animation if restart animation is set, change current request's current URL to urlString, and then fire a simple event named load at the img element.
8. Abort the update the image data algorithm.


And different algorithm:
1. If image request is the pending request, abort the image request for the current request, upgrade the pending request to the current request and update the img element's presentation appropriately.
2. Set image request to the completely available state.
3. Add the image to the list of available images using the key key, with the ignore higher-layer caching flag set.
4. Fire a progress event or simple event named load at the img element, depending on the resource in image request.

In both cases it says presentation is updated before 'load'.
And in the first case in particular there is async step after updating image presentation, so animation frame may happen before load event is dispatched.

Whether or not the spec is good is then another question.


https://html.spec.whatwg.org/#update-the-image-data

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

So, image should have got the new presentation before load.

But it is unclear to me what the test is testing. The presentation is possibly updated before load, but the test just seems to test that load happens before animation frame tick, and not testing anything related to old or new image.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Oh, wait yes, now I see. There is first color 1 visible, then some movement and then color 2.

So, you say this is a regression. Regression range would be really nice.


seth, does this ring any bells?

Comment 4

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Edgar, do you have ideas of what's up here? I tried to use mozregression tool to find the regression range, here it is: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fc15477ce628599519cb0055f52cc195d640dc94&tochange=37f04460ddb76d6ef4e7c32a8a6b2fbc44cb8776  (although I am not super confident about the range I got, since the issue seems not reproducible all the time so ...)

Comment 5

[Edgar Chen [:edgar]]

mozregression result from my side:

 8:20.72 INFO: Last good revision: b12d4b8634f3e1997dcaca3ac742c5ea3ea12df1
 8:20.72 INFO: First bad revision: 3e12df7cefe4cf2e0b8dc11867e1008b21aa135a
 8:20.72 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b12d4b8634f3e1997dcaca3ac742c5ea3ea12df1&tochange=3e12df7cefe4cf2e0b8dc11867e1008b21aa135a

 8:21.67 INFO: Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1253995

Comment 6

[Edgar Chen [:edgar]]

I backout bug 1253995 on the top of latest m-c locally, and then I cannot reproduce this issue. So I am pretty sure this is a regression of bug 1253995.