1862235 - PNG with out-of-range palette indices in tRNS incorrectly handled

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[chris]

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

Steps to reproduce:

Run http://wpt.live/png/trns-chunk.html

Actual results:

Fail

Expected results:

Pass

Test passes in Chrome (and Edge) and Safari.

Comment 1

[chris]

The PNG specification had an ambiguity in the Second Edition:
https://www.w3.org/TR/2003/REC-PNG-20031110/#11tRNS

If the image bit depth is less than 16, the least significant bits are used and the others are 0.

After a bug report
https://github.com/w3c/PNG-spec/issues/96

this was clarified in Third Edition
https://w3c.github.io/PNG-spec/#11tRNS

If the image bit depth is less than 16, the least significant bits are used. Encoders should set the other bits to 0, and decoders must mask the other bits to 0 before the value is used.

The referenced WPT tests specifically for this updated text.

Comment 2

[Monica Chiorean]

Put link from description on Win11x64 using FF build 120.0b4 and it failed, while on Chrome passed.
Marking issue as new, not sure if component is correct please update if otherwise. Thank you.

Comment 3

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

The severity field is not set for this bug.
:jteow, could you have a look please?

For more information, please visit BugBot documentation.

Comment 4

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

The severity field is not set for this bug.
:tnikkel, could you have a look please?

For more information, please visit BugBot documentation.

Comment 5

[lucas.chollet]

Attachment: Bug 1862235 Remove non-compliant behavior with tRNS chunks. r=tnikkel

In previous editions of the spec, out-of-range values were not
considered. As this is now specified, we should let libpng do the right
thing and not inject non-compliant behavior.

Comment 6

[lucas.chollet]

I started to investigate this issue, and there is code in nsPNGDecoder.cpp that was purposely written to have the unwanted behavior. It seems that there used to be some PNG images that had out-of-range palette indices in the wild, and that it was chosen to discard transparency in this case (bug #428045). As mentioned above, this is not spec compliant with regard to the third edition.

Removing these few lines makes the bug disappear and Firefox pass the WPT test, thus compliant to the newest PNG standard.

As you can see, I submitted a patch for it. I'm hoping I did nothing wrong, this is my first contribution here.

Comment 7

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 8

[chris]

Wondering what happened to the patch, can someone review it?

Firefox is the only browser to fail this WPT, Chrome Edge and Safari all pass.

https://wpt.fyi/results/png/trns-chunk.html?label=experimental&label=master&aligned

Comment 9

[chris]

And the patch is literally to remove an extra check that Mozilla added (years ago when the spec was unclear) now that the spec is clear, restoring compatibility with the other browsers.

Comment 10

[chris]

Chrome Edge and Safari all pass.

Actually Chrome, Edge, Safari, Ladybird and Servo all pass
https://wpt.fyi/results/png/trns-chunk.html?label=master&product=chrome%5Bexperimental%5D&product=edge%5Bexperimental%5D&product=firefox%5Bexperimental%5D&product=safari%5Bexperimental%5D&product=servo&product=ladybird&aligned

Comment 11

[chris]

:emilio how can we unblock this? It is a very trivial patch and all the other browsers pass.

Comment 12

[Emilio Cobos Álvarez [:emilio]]

I pinged Tim. I also think the test expectation should ideally be updated. Pushed to try: https://treeherder.mozilla.org/jobs?repo=try&revision=95532bd51ee3078ef338062c5b16ba852052fcce

Comment 13

[Timothy Nikkel (:tnikkel)]

Okay, reviewed. The metadata needs to be updated to land though.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ead0654e7a2
Remove non-compliant behavior with tRNS chunks. r=tnikkel

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/3ead0654e7a2