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

[Gecko Bug 1684909] Add and adjust image-set tests / test expectations. #27167

Merged
merged 1 commit into from Jan 13, 2021

Conversation

moz-wptsync-bot
Copy link
Collaborator

The tests uncover two things:

  • cursor: image-set() doesn't work, because there's no in the
    spec: https://drafts.csswg.org/css-ui-4/#cursor

  • content: image-set() similarly doesn't work. Though in this case the
    spec does call for it to work.

The later doesn't work because we don't support the whole syntax
in content (so, we don't support content: linear-gradient or so on).
That seems like a separate issue to fix (WebKit does support this).

The former should probably also work (though WebKit only supports URLs
and not other images like gradients... will file a spec issue).

It also caught a bug with first-contentful-paint which I fixed drive-by:
image-set(url(..)) should be considered contentful, just like url(..) is.

The test changes are mostly straight-forward, but two changes are worth
mentioning:

  • I moved one subtest that tested content: image-set() from
    image-set-rendering.html to its own test, so that we can annotate the test
    as passing and ensure it doesn't regress (there's no way to annotate a
    reftest as "partially passing").

  • I changed a test that was testing image-set('/images/red.png') as invalid
    incorrectly (resolution is optional per spec and behaves like 1x).

Differential Revision: https://phabricator.services.mozilla.com/D100701

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1684909
gecko-commit: 5c7363d33df344a148a63aff09f9c548bbc144ac
gecko-reviewers: dholbert

The tests uncover two things:

 * cursor: image-set() doesn't work, because there's no <image> in the
   spec: https://drafts.csswg.org/css-ui-4/#cursor

 * content: image-set() similarly doesn't work. Though in this case the
   spec does call for it to work.

The later doesn't work because we don't support the whole <image> syntax
in `content` (so, we don't support `content: linear-gradient` or so on).
That seems like a separate issue to fix (WebKit does support this).

The former should probably also work (though WebKit only supports URLs
and not other images like gradients... will file a spec issue).

It also caught a bug with first-contentful-paint which I fixed drive-by:
image-set(url(..)) should be considered contentful, just like url(..) is.

The test changes are mostly straight-forward, but two changes are worth
mentioning:

 * I moved one subtest that tested content: image-set() from
   image-set-rendering.html to its own test, so that we can annotate the test
   as passing and ensure it doesn't regress (there's no way to annotate a
   reftest as "partially passing").

 * I changed a test that was testing image-set('/images/red.png') as invalid
   incorrectly (resolution is optional per spec and behaves like 1x).

Differential Revision: https://phabricator.services.mozilla.com/D100701

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1684909
gecko-commit: 5c7363d33df344a148a63aff09f9c548bbc144ac
gecko-reviewers: dholbert
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Firefox project.

@moz-wptsync-bot moz-wptsync-bot merged commit ec1f357 into master Jan 13, 2021
@moz-wptsync-bot moz-wptsync-bot deleted the gecko/1684909 branch January 13, 2021 16:45
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

3 participants