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

Fix thresholds definition to match browsers. #516

Merged
merged 1 commit into from
Oct 3, 2023
Merged

Conversation

emilio
Copy link
Collaborator

@emilio emilio commented Oct 2, 2023

web-platform-tests/wpt@8c0296b tested that thresholds when provided empty is [0].

This is not in the spec, but is reasonable and I'm happy to change Firefox to match this, given both WebKit and Blink do that.

The following tasks have been completed:

  • Modified Web platform tests: N/A, WPT already tests this.

Implementation commitment:


Preview | Diff

web-platform-tests/wpt@8c0296b
tested that thresholds when provided empty is [0].

This is not in the spec, but is reasonable and I'm happy to change
Firefox to match this, given both WebKit and Blink do that.
Copy link
Collaborator

@tcaptan-cr tcaptan-cr left a comment

Choose a reason for hiding this comment

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

Looks good to me

@emilio emilio merged commit e1c3797 into main Oct 3, 2023
2 checks passed
@emilio emilio deleted the empty-sequence branch October 3, 2023 17:30
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 3, 2023
…. r=smaug

Technically a broken test, but given other browsers' behavior and that
having no thresholds is useless, I don't think this edge-case is worth
fighting about.

See w3c/IntersectionObserver#516 for the spec
change.

Differential Revision: https://phabricator.services.mozilla.com/D189764
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Oct 5, 2023
…. r=smaug

Technically a broken test, but given other browsers' behavior and that
having no thresholds is useless, I don't think this edge-case is worth
fighting about.

See w3c/IntersectionObserver#516 for the spec
change.

Differential Revision: https://phabricator.services.mozilla.com/D189764
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants