1779791 - png does not display

Status: UNCONFIRMED

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

Description

[15063662]

Attachment: animated.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Firefox/102.0

Steps to reproduce:

1. Download a sample APNG at
   https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png
2. Open with MS Paint and edit (fill a pixel with black in the attachment)
3. Open with Firefox

Actual results:

Firefox:
The image "file://.../animated.png" cannot be displayed because it contains errors.
Image viewer:
A static ball with a black pixel.
Chrome:
A static ball with a black pixel.

Expected results:

Image should be displayed (A static ball with a black pixel).

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox::File Handling' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Timothy Nikkel (:tnikkel)]

Turning on pngdecoder logging I get the following for the attachment (but not for the wikipedia original image):

W/PNGDecoder libpng warning: Duplicate fcTL within one frame skipped
E/PNGDecoder libpng error: fcTL or fdAT chunk with out-of-order sequence number found

Comment 3

[Timothy Nikkel (:tnikkel)]

Structure of the original wikipedia image:

IHDR
acTL
fcTL
IDAT
fcTL
fdAT
...
fcTL
fdAT
IEND

Structure of the attchment:

IHDR
sRGB
gAMA
pHYs
acTL
fcTL
fcTL
fdAT
...
fcTL
fdAT
IDAT
IEND

So it looks like MS Paint is not apng aware and it just moves/saves the edited IDAT at/to the end of the file, which messes up the apng sequence numbering and gives us two fcTL in a row at the start where the IDAT used to be. Per spec https://wiki.mozilla.org/APNG_Specification these are supposed to be rejected. But it seems like we could be more permissive in allowing this kind of error.

Comment 4

[Timothy Nikkel (:tnikkel)]

Interestingly, although Chrome fails back to rendering the file as a static PNG, Safari is still able to render it as animated.

If I comment out this code then we get the Chrome behaviour (with a bunch of warnings output in the pngdecoder mozlog)
https://searchfox.org/mozilla-central/rev/99c3ccc2f02c5fff7c9850f7a0770ee92cc693f6/media/libpng/pngrutil.c#3032

Comment 5

[Jim Mathies [:jimm]]

(In reply to Timothy Nikkel (:tnikkel) from comment #3)

So it looks like MS Paint is not apng aware and it just moves/saves the edited IDAT at/to the end of the file, which messes up the apng sequence numbering and gives us two fcTL in a row at the start where the IDAT used to be. Per spec https://wiki.mozilla.org/APNG_Specification these are supposed to be rejected. But it seems like we could be more permissive in allowing this kind of error.

Tim, what do you suggest we do here? Should we land the change you suggested?

Setting priority and adding to gfx-triage. Waiting for feedback from Tim.

Comment 6

[Timothy Nikkel (:tnikkel)]

(In reply to Jim Mathies [:jimm] from comment #5)

Tim, what do you suggest we do here? Should we land the change you suggested?

I'm not sure we want to make that specific code change, it was more of an experiment. That would require changing the error behaviour internally of libpng.

It would be better if we could change our png decoder to just accept this specific error and continue instead of terminating. To do that properly we'd want to make sure that the control flow our of decoder would not be able to get confused by allowing this new behaviour.

Comment 7

[Kelsey Gilbert [:jgilbert]]

It sounds like we want to handle this at least as gracefully as Chrome does.