802390 - alt text is shown simultaneously with images

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[(dormant account)]

Attachment: corruption on amazon

Just came back to my browser and my inactive tabs ended up with alt-text all over some of the images. Attached is a screenshot on amazon. This state persists until ctrl-shift-r. I think a simple reload did not help.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: on facebook

I hit this as well, a few days ago, when switching to a facebook tab.

The image in this screenshot is supposed to be just one tongue-sticking-out smiley-face, but it ended up (a) showing the alt text, and (b) changing the image bounds to be as large as the alt text.  (making it show additional image-sprite content -- a whole other smiley-face)

Comment 2

[Daniel Holbert [:dholbert]]

(FWIW, my screenshot was taken on Sunday Oct 14, using an up-to-date Linux 64-bit nightly)

Comment 3

[(dormant account)]

Mine is from today's nightly

Comment 4

[(dormant account)]

(In reply to Taras Glek (:taras) from comment #3)
> Mine is from today's nightly

Today's nightly is still suffering, but i have STR now. open up gmail, plus an image-heavy tab such as flickriver.com...scroll around in flickriver.com enough to evict gmail image cache entries, then switch to gmail tab and watch alt-text appear dom elements. Note the alt-text appears on all new image-elements inserted into the dom(even if they use the same images as are already displayed elsewhere), old ones are fine and reloading the page makes all images show alt-text. ctrl-shift-r resets the cache and alt-text disappears.

Comment 5

[(dormant account)]

(In reply to Taras Glek (:taras) from comment #4)
> (In reply to Taras Glek (:taras) from comment #3)
> > Mine is from today's nightly
> 
> Today's nightly is still suffering, but i have STR now. open up gmail, plus
> an image-heavy tab such as flickriver.com...scroll around in flickriver.com
> enough to evict gmail image cache entries, then switch to gmail tab and
> watch alt-text appear dom elements. Note the alt-text appears on all new
> image-elements inserted into the dom(even if they use the same images as are
> already displayed elsewhere), old ones are fine and reloading the page makes
> all images show alt-text. ctrl-shift-r resets the cache and alt-text
> disappears.

For a while I could reproduce this with yesterday's build, but then the issue ceased to be reproducable so my STR is wrong. I talked to dholbert and looks like the only sites that exhibit this issue are spdy sites: facebook, amazon, twitter, gmail.

Comment 6

[Patrick McManus [:mcmanus]]

(In reply to Taras Glek (:taras) from comment #5)


> 
> For a while I could reproduce this with yesterday's build, but then the
> issue ceased to be reproducable so my STR is wrong. I talked to dholbert and
> looks like the only sites that exhibit this issue are spdy sites: facebook,
> amazon, twitter, gmail.

fwiw I don't believe amazon.com uses any sdpy and facebook uses very little at this point in time in production (though they are testing it).

might be a cache issue.

Comment 7

[Honza Bambas (:mayhemer)]

Why is this actually in the networking component?  It seems more to be some layout or rendering issue...

Comment 8

[(dormant account)]

(In reply to Honza Bambas (:mayhemer) from comment #7)
> Why is this actually in the networking component?  It seems more to be some
> layout or rendering issue...

Feels like a networking issue because resetting the cache fixes it but a simple reload doesn't.

Comment 9

[Josh Aas]

Michal - Taras thinks this might have something to do with the cache state for images. He's guessing that maybe the cache is reporting that images are incomplete. Assigning to you to investigate, we can come up with a new plan if you determine that this isn't a cache bug.

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: on jobvite (broken-image icon shown simultaneously w/ real image)

Just hit something that looks like this bug on the https://hire.jobvite.com/ login page, after having a few jobvite tabs open for a few hours (long enough to make my Jobvite session time out and redirect me to the login page)

In this case, it showed the broken-image icon instead of alt text, but I suspect it's the same issue.  (it's just that we don't have alt-text to show for the images in this case.  So the underlying issue that makes us show the alt-text on the other sites ends up making us show a broken-image icon at Jobvite.)

Comment 11

[Timothy Nikkel (:tnikkel)]

This bug seems very similar (possibly the same) as bug 804000, for which the regressing changeset is known.

Comment 12

[Daniel Holbert [:dholbert]]

I'm currently hitting this with the green/red availability indicator icons in the contact list on the left side of my Gmail tab.

From inspecting those elements, they're effectively
 <img src="https://mail.google.com/mail/u/0/images/cleardot.gif"
      alt"some text"
      style="background: https://ssl.gstatic.com/ui/v1/icons/mail/icons2.png">

where that background image is a sheet of sprites.

If I try to view either cleardot.gif or icons2.png directly, I get an error page:
> The image [url] cannot be displayed because it displays errors.

Comment 13

[Joe Drew (not getting mail)]

Attachment: handle shutting down decoders before any work has been done

Daniel set a watchpoint on a particular image going bad, and using that we figured out when this happens.

It turns out to be the case that we don't handle shutting down a decoder that hasn't had a chance to do any work yet. The intention of this code is to let us display animated images that go bad after we've had at least one good frame. Unfortunately it didn't actually test whether things went bad, and just used "I have at least one frame" as a proxy for that.

Making things explicit should make this bug go away.

Comment 14

[Joe Drew (not getting mail)]

Attachment: handle shutting down decoders before any work has been done

now with less patch

Comment 15

[Joe Drew (not getting mail)]

Attachment: handle data errors more correctly

jeff reviewed this over my shoulder

Comment 16

[Joe Drew (not getting mail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fb3e2fb82000

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Unfortunately, this caused mochitest browser-chrome failures on all platforms. Backed out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/84384977f87b

https://tbpl.mozilla.org/php/getParsedLog.php?id=17114895&tree=Mozilla-Inbound

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/webconsole/test/browser_webconsole_bug_595934_message_categories.js | Timed out while waiting for: test #15 successful finish
Stack trace:
    JS frame :: chrome://mochitests/content/browser/browser/devtools/webconsole/test/head.js :: wait :: line 294
    JS frame :: chrome://mochitests/content/browser/browser/devtools/webconsole/test/head.js :: <TOP_LEVEL> :: line 304
    native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

Comment 19

[Joe Drew (not getting mail)]

Attachment: expose the shutdown intent to Decoder::Finish()

Most of our time was spent trying to reverse-engineer what type of decode shutdown we were doing, but that broke things for various reasons. Instead of reverse-engineering it, let's just expose it directly.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=73e4455ba8f4

Comment 20

[Joe Drew (not getting mail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/305129d04cd1

I'll ask for aurora uplift once this has proven itself.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/305129d04cd1

Comment 22

[Lukas Blakk [:lsblakk] use ?needinfo]

Triage drive-by: not asking for uplift yet since comment 20 suggests Joe will do the right thing soon...

Comment 23

[Joe Drew (not getting mail)]

Comment on attachment 685310 [details] [diff] [review]
expose the shutdown intent to Decoder::Finish()

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 792199
User impact if declined: Images will occasionally fail to decode on quick tab switches.
Testing completed (on m-c, etc.): On m-c for a week
Risk to taking this patch (and alternatives if risky): Probably not too risky. Could expose other bugs, but probably won't.
String or UUID changes made by this patch: none

Comment 24

[Alex Keybl [:akeybl]]

Comment on attachment 685310 [details] [diff] [review]
expose the shutdown intent to Decoder::Finish()

Given where we are in the cycle, and the fact that we've got an extra week of Aurora here, approving for uplift. This is manageable risk.

Comment 25

[Joe Drew (not getting mail)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/568804ee53fd

The offending patch was actually disabled on 19; should we set the status-firefox19 flag to disabled?

Comment 26

[Joe Drew (not getting mail)]

Er, s/19/18. And ignore me, I can't read. Gah.

Comment 27

[Bogdan Maris, Desktop Test Engineering]

Mozilla/5.0 (X11; Linux x86_64; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0

Did not encountered any issues using Firefox 20.0 (buildID: 20130326150557).