1395964 - Image flash when current source changes

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect, P2)

Description

[oliverjash]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.25 Safari/537.36

Steps to reproduce:

Example: http://output.jsbin.com/lacufep/2

In the example, click the image. This will modify the `sizes` attribute, forcing the evaluated source to change.

Alternatively:

1. Create a responsive image
2. Trigger a change to the parsed image size (from `sizes` attribute; e.g. resize the document), forcing the evaluated `currentSrc` to change


Actual results:

Visually, the old image source is replaced with the new one, i.e. the current source is changed, however there is a brief flash in between the old source showing and the new source being shown.


Expected results:

There should be no flash, as this is distracting for users.

Comment 1

[Alice0775 White]

(FYI, It is considerably improved if setting image.decode-immediately.enabled=true. And the issue disappear if disabling e10s and setting image.decode-immediately.enabled=true both.)

Comment 2

[Alice0775 White]

At least, Regression window of the problem(the entire image once becomes white) is as follows. 

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8c07b084c0f8fb17885979c3322f5e15a1298bbe&tochange=aafd6db2fbb452350c42a1d839ce96ec6e183b53

Regressed by: Bug 1207355


Although there is a small flicker(bottom part of image) sometimes even before the landing of Bug 1207355.

Comment 3

[Liz Henry (:lizzard) (relman/hg->git project)]

Too late for a fix for 57 since we're a few days away from release.

Comment 4

[Firefox Bug Husbandry Bot]

https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

Too late to fix in 63, but we could still take a patch in 65 and potentially, 64 beta.

Comment 6

[Andreas Bovens [:abovens]]

Sean, putting this on your radar. It might affect other similar sites as well.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Bug 1253995 looks related. Probably we're clearing mPrevImage too early, or something like that...

Comment 8

[Sean Voisen (:svoisen)]

Given this is a webcompat bug that affects the Google image search user experience, I'm going to bump to P2 and see if we can get some movement on it.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

Bug 1253995 looks related. Probably we're clearing mPrevImage too early, or
something like that...

The reason this happens is because each time we change the load state we reframe the image, loosing the previous image of course:

https://searchfox.org/mozilla-central/rev/00f3836a87b844b5e4bc82f698c559b9966e4be2/layout/base/RestyleManager.cpp#490

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Tentatively taking.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1395964 - Make image loading changes not flash. r=tnikkel

There are two parts for this fix.

The first one is trivial, and consists on not reframing the image when the
loading state changes. Bug 1472637 makes the decision of whether to construct an
image frame not depend on this, so this is sound.

We need to avoid reframing because otherwise we lose track of the previously
painted image.

The second part is not clearing mPrevImage if our intrinsic size changes.

This is needed if we want to handle cases like the reporter's example or Google
Images. Rendering the old image upscaled is much better than not rendering
anything while it's loading.

I added a test for the reframing bit, but I don't know how to add a test for the
second bit.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f1e9dae2467
Make image loading changes not reframe. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/fdc7b66d76a4
Don't clear the previously painted image on intrinsic size changes. r=tnikkel

Comment 13

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/3f1e9dae2467
https://hg.mozilla.org/mozilla-central/rev/fdc7b66d76a4

Comment 14

[Pascal Chevrel:pascalc (PTO until April 26)]

We have shipped multiple releases with this bug, let's make the fix the trains, marking 67 as wontfix.