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

New tests for use of currentColor in @font-palette-values #31517

Conversation

faceless2
Copy link
Contributor

@faceless2 faceless2 commented Nov 5, 2021

New tests to verify use of currentColor in @font-palette-values override-colors property.

font-palette-36.html makes the assumption that currentColor is inherited in the same way here as everywhere else, but this isn't explicit in the spec.

ping @litherum

@faceless2 faceless2 changed the title New tets for use of currentColor in @font-palette-values New tests for use of currentColor in @font-palette-values Nov 5, 2021
Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

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

LGTM and I agree that use of currentColor in @font-palette-values is underspecified, but the approach assumed in this test seems reasonable

@svgeesus svgeesus merged commit e14ed86 into web-platform-tests:master Nov 18, 2021
@faceless2 faceless2 deleted the bfo-font-palette-currentcolor-test branch November 18, 2021 13:39
@litherum
Copy link
Contributor

Uh-oh, I think this is opposite from our implementation 🤔

@drott
Copy link
Contributor

drott commented Feb 10, 2022

In w3c/csswg-drafts#7010 @litherum explains that currentColor is intentionally omitted. Example 81 has currentColor, but the spec text had <<absolute-color>> and is getting revised in w3c/csswg-drafts#7010. With that, I believe, a test for currentColor would not be valid.

From an implementation perspective, I find it difficult to resolve currentColor at the time of font instantiation, or vice versa: The time of being able to resolve currentColor is not the time of when the font is getting instantiated. For the purpose of current color usage, I suggest a COLRv0/COLRv1 font that uses color index 0xFFFF.

Safari currently does not pass this test, and in my WIP implementation in Chrome, I find this hard to support.

@svgeesus
Copy link
Contributor

OK so with that clarification, these tests should be deleted.

@faceless2
Copy link
Contributor Author

Fine by me to delete these as they don't match the current wording. Agree it is considerably more complex using currentColor in font-palette-values, that's why I made these tests.

I still think it's important it's both supported and consistent with the way it works elsewhere in terms of inheritance, but that's better discussed in the issue than here.

svgeesus pushed a commit that referenced this pull request Feb 13, 2022
mattwoodrow pushed a commit to mattwoodrow/wpt that referenced this pull request Feb 15, 2022
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 26, 2022
… in @font-palette-values", a=testonly

Automatic update from web-platform-tests
Revert "New tets for use of currentColor in @font-palette-values"

This reverts commit e14ed8613f431e528b83f074ce37cf6e22dd95b6.

As discussed in
web-platform-tests/wpt#31517 and
w3c/csswg-drafts#7010

--

wpt-commits: 13b16f2df09ae8b579fbf9ebeca5f6153f494d55
wpt-pr: 32812
DanielRyanSmith pushed a commit to DanielRyanSmith/wpt that referenced this pull request Feb 28, 2022
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Mar 1, 2022
… in @font-palette-values", a=testonly

Automatic update from web-platform-tests
Revert "New tets for use of currentColor in @font-palette-values"

This reverts commit e14ed8613f431e528b83f074ce37cf6e22dd95b6.

As discussed in
web-platform-tests/wpt#31517 and
w3c/csswg-drafts#7010

--

wpt-commits: 13b16f2df09ae8b579fbf9ebeca5f6153f494d55
wpt-pr: 32812
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Mar 8, 2022
… in @font-palette-values", a=testonly

Automatic update from web-platform-tests
Revert "New tets for use of currentColor in @font-palette-values"

This reverts commit e14ed8613f431e528b83f074ce37cf6e22dd95b6.

As discussed in
web-platform-tests/wpt#31517 and
w3c/csswg-drafts#7010

--

wpt-commits: 13b16f2df09ae8b579fbf9ebeca5f6153f494d55
wpt-pr: 32812
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Mar 8, 2022
… in @font-palette-values", a=testonly

Automatic update from web-platform-tests
Revert "New tets for use of currentColor in @font-palette-values"

This reverts commit e14ed8613f431e528b83f074ce37cf6e22dd95b6.

As discussed in
web-platform-tests/wpt#31517 and
w3c/csswg-drafts#7010

--

wpt-commits: 13b16f2df09ae8b579fbf9ebeca5f6153f494d55
wpt-pr: 32812
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.

None yet

6 participants