Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test priority of cICP over iCCP #44644

Merged
merged 2 commits into from
Feb 21, 2024
Merged

Conversation

svgeesus
Copy link
Contributor

PNG Third edition says:

When the cICP chunk is present, decoders that recognize it SHALL ignore the following chunks:

iCCP

The test file is a PNG with solid sRGB green (0, 128, 0) converted to Display P3 (0.21603749037034126, 0.49418159073224005, 0.13151307826427636). The image has a Display P3 cICP chunk. It also has an iCCP chunk containing a modified sRGB ICC profile; the red and green colorants are swapped so, if used, the result will be a deep red.

The test HTML also starts with a solid red, in case the image is not loaded at all.

The reference uses an sRGB PNG which is sRGB green (0, 128, 0).

@wpt-pr-bot
Copy link
Collaborator

There are no reviewers for this pull request besides its author. Please reach out on the chat room to get help with this. Thank you!

@svgeesus
Copy link
Contributor Author

@ProgramMax

@svgeesus svgeesus mentioned this pull request Feb 19, 2024
@ProgramMax
Copy link
Contributor

LGTM, except for the lint error.

It says you need a link to the spec. You have two, though. I'm not sure why it doesn't recognize them.

@svgeesus
Copy link
Contributor Author

Yeah I don't understand the lint error either, especially as it mentions an entirely unrelated spec:

css/css-highlight-api/historical.window.js: Testcase file must have a link to a spec (MISSING-LINK)

@svgeesus
Copy link
Contributor Author

@web-platform-tests/wpt-core-team any help on resolving this apparently unrelated lint error?

@zcorpan
Copy link
Member

zcorpan commented Feb 20, 2024

Rebase to the latest master (or any commit after b31783e )

@svgeesus
Copy link
Contributor Author

@ProgramMax you have been added to the reviewer pool, please approve

Copy link
Contributor

@ProgramMax ProgramMax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Ms2ger Ms2ger merged commit 0578952 into web-platform-tests:master Feb 21, 2024
19 checks passed
marcoscaceres pushed a commit that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants