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

@moz-wptsync-bot moz-wptsync-bot commented Jan 13, 2021

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

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
24 checks passed
24 checks passed
update-pr-preview
Details
update-pr-preview
Details
update-pr-preview
Details
update-pr-preview
Details
update-pr-preview
Details
Azure Pipelines Build #20210113.28 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
download-firefox-nightly Community-TC (pull_request)
Details
lint Community-TC (pull_request)
Details
sink-task Community-TC (pull_request)
Details
upstream/gecko Landed on mozilla-central
Details
wpt-chrome-dev-results Community-TC (pull_request)
Details
wpt-chrome-dev-results-without-changes Community-TC (pull_request)
Details
wpt-chrome-dev-stability Community-TC (pull_request)
Details
wpt-decision-task Community-TC (pull_request)
Details
wpt-firefox-nightly-results Community-TC (pull_request)
Details
wpt-firefox-nightly-results-without-changes Community-TC (pull_request)
Details
wpt-firefox-nightly-stability Community-TC (pull_request)
Details
wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - firefox[experimental] Firefox results
Details
@moz-wptsync-bot moz-wptsync-bot deleted the gecko/1684909 branch Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants