1472637 - images flash alt attribute while being fetched

Status: RESOLVED FIXED

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

Description

[Jens]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0
Build ID: 20180701220749

Steps to reproduce:

1. I created a minimal reproducable example at https://codepen.io/nachtigall/full/OEdYZQ/

When loading this page, try to throttle network speed in DevTools' network tab and disable Cache.


Actual results:

The image `alt` attribute's text is flashed to the user when the image gets loaded. This causes ugly kind of Flash of Unstyled Content (FOUC) problems. The more and the smaller the images, the uglier it looks in Firefox.


Expected results:

The `alt` text should not flash up while the image being fetched. Like Chrome and Edge do it.

Comment 1

[Jens]

FWIW, happens in Firefox 60, 61 and Nightly 63. Under Linux and Windows.

Comment 2

[Jens]

Here's a correspondent Stackoverflow question where another user sees the same problem together with a workaround hack: https://stackoverflow.com/questions/12667926/hide-alt-tag-in-firefox 

I think Firefox should behave the same way like Chrome or Edge, there's no point in showing the `alt` text when the image is just going to be loaded.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

This is per spec, fwiw, see bug 1196668 for recent fun. Issue is, WebKit and Blink do nothing close to the spec :(

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Hmm, though actually, the visibility: hidden bit should prevent that. What makes us reconstruct the frame when src is added?

I'd have expected something like: https://searchfox.org/mozilla-central/rev/60c4067b1cbb0f94d7dc2d7cdfa27ed579817fee/dom/html/HTMLImageElement.cpp#234 but for src instead of alt...

Let me look more carefully into this tomorrow.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Oh, never mind, I know how this works. I think this shouldn't be too terrible to fix, though a bit gross.

So the issue is that the broken state is unset too late, when the image load task runs and we call LoadImage(), and there's a small period of time when both the broken state and the src attributes are set.

A workaround you could use is to also do:

img:-moz-loading {
  visibility: hidden;
}

But probably we should just not display the alt-text when the image is loading, which is:

https://searchfox.org/mozilla-central/rev/60c4067b1cbb0f94d7dc2d7cdfa27ed579817fee/layout/style/res/html.css#646

Thanks for the report Jens, as always.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1472637 - Don't display alt text while loading, to match other UAs.

Not 100% sure we want to do this, but I'd rather submit the patch...

The use-case is valid, but on the other hand it can also be solved listening to
load events or what not.

Anyhow, your call in terms of whether we want to take it or not.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

This also needs quite a few fixes to our tests if we decide we want this...

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Boris, would you be fine with doing something like this?

As I mentioned in the commit message the use-case is valid, I think, but on the other hand it can also be solved in other ways, and if you have a very slow connection showing the alt text while the image loads may pretty much be a feature...

So not sure? :)

Comment 9

[Boris Zbarsky [:bzbarsky]]

So let me see whether I understand this correctly.

The image starts off with no src but with alt text. In that situation, the correct thing to do is to show the alt text. At this point the image is doing just that, but is visibility:hidden.

Then the script on the page sets an src on the image. At this point we kick off the image load. That should change the content state, and trigger a potential frame reconstruct.

In any case, assuming that this triggers a reframe, we land in nsImageFrame::ShouldCreateImageFrameFor. This returns false because:

• We are in the LOADING state, so IMAGE_OK only returns true if there's a specified size.
• There is no specified size.

I think the idea was in fact that on really slow networks at least the alt text is readable until the image manages to load... But I also thought that we started images off with an image frame initially, to avoid expensive reframes. Clearly we don't... and removing the LOADING state will reframe anyway.

In any case, in spec terms https://html.spec.whatwg.org/multipage/rendering.html#images-3 what should happen is interesting. Going through the list one by one:

1. Does not apply, since per https://html.spec.whatwg.org/multipage/embedded-content.html#the-img-element the <img> currently represents the alt text.

2. Does not apply, because there is no size specified.

3. Arguably does not apply, because we are expecting the "represents text" bit to change (!).

4. Does not apply, since the <img> represents text.

5. Does not apply, because we do expect the "does not represent an image" bit to change.

Anyway, the simplest thing to do would be to change what ShouldCreateImageFrameFor does in the LOADING state, as well as update the spec to actually cover this case. In particular, seems like the "user agent has reason to believe that the image will become available" case should not be conditioned on having specified size.

Comment 10

[Boris Zbarsky [:bzbarsky]]

That said, even if we did the imageframe thing, you would still get flashes of stuff due to the missing size information: the images would start off very small and then resize once the size is available...

Comment 13

[Emilio Cobos Álvarez (:emilio)]

I finally took a bit of time to understand the red:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d26e9b31fc68e0402fbd60f5bccaf876bcf8fc6

Is much greener :)

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #9)

Anyway, the simplest thing to do would be to change what ShouldCreateImageFrameFor does in the LOADING state, as well as update the spec to actually cover this case. In particular, seems like the "user agent has reason to believe that the image will become available" case should not be conditioned on having specified size.

I agree, and that's what my patch does. I'll keep my ni? to send a PR to the HTML spec.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Sent https://github.com/whatwg/html/pull/4406

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/94682e23ba11
Don't display alt text while loading, to match other UAs. r=tnikkel

Comment 16

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/94682e23ba11

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16009 for changes under testing/web-platform/tests

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging