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.

Sign up for GitHub

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

Jump to bottom

Use FrozenArray<double> for IntersectionObserver's thresholds attribute #114

Closed

foolip opened this issue Mar 11, 2016 · 9 comments

Assignees

Labels

Member

foolip commented Mar 11, 2016 •

edited

Loading

https://wicg.github.io/IntersectionObserver/#intersection-observer-interface

This is very similar to Notification's vibrate and action attributes:
https://notifications.spec.whatwg.org/#api

As it is, this would return a new array every time.

@annevk @bzbarsky @domenic

The text was updated successfully, but these errors were encountered:

domenic commented Mar 11, 2016

This is important. Please do not ship thresholds in the current state.

Member Author

foolip commented Mar 11, 2016

Right, I got here from Intent to Ship: Intersection Observer on blink-dev.

Collaborator

slightlyoff commented Mar 31, 2016

It's unclear why a new array each time is problematic in this case. Is the concern allocation or some other semantic issue (e.g. side-channels)?

Member

annevk commented Mar 31, 2016

Semantic. It's important to preserve x.prop === x.prop.

Collaborator

ojanvafai commented Mar 31, 2016

Why is it important? (honest question)

domenic commented Mar 31, 2016

https://w3ctag.github.io/design-principles/#attributes-like-data

Member Author

foolip commented Sep 23, 2016

Ping? Blink's IDL file already uses FrozenArray<double> for this.

Member Author

foolip commented Jan 11, 2017

Ping again.

Collaborator

ojanvafai commented Feb 9, 2017

@szager-chromium looks like the spec here needs updating.

ojanvafai added the needs spec label

ojanvafai assigned szager-chromium

szager-chromium mentioned this issue

Switch IntersectionObserver.threshold to a FrozenArray. #289

Merged

szager-chromium closed this as completed

Ahmad-S792 mentioned this issue

Use 'FrozenArray' for 'thresholds' in IntersectionObserver.idl WebKit/WebKit#16653

Merged

webkit-commit-queue pushed a commit to Ahmad-S792/WebKit that referenced this issue


          Use 'FrozenArray' for 'thresholds' in IntersectionObserver.idl

37667ad

https://bugs.webkit.org/show_bug.cgi?id=260129

Reviewed by Ryosuke Niwa.

This patch is to align WebKit with Web-Spec [1] and Blink / Chromium.

[1] https://www.w3.org/TR/intersection-observer/#intersection-observer-interface

GitHub Issue: w3c/IntersectionObserver#114

* Source/WebCore/page/IntersectionObserver.idl: Update
* LayoutTests/imported/w3c/web-platform-tests/intersection-observer/idlharness.window-expected.txt: Rebaselined

Canonical link: https://commits.webkit.org/266857@main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment