Bug 260129 - Use 'FrozenArray' for 'thresholds' in IntersectionObserver
Summary: Use 'FrozenArray' for 'thresholds' in IntersectionObserver
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL: http://wpt.live/intersection-observer...
Keywords: BrowserCompat, InRadar, WPTImpact
Depends on:
Blocks:
 
Reported: 2023-08-13 13:42 PDT by Ahmad Saleem
Modified: 2023-08-13 17:16 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2023-08-13 13:42:57 PDT
Hi Team,

Currently we are failing one IDLHarness test for Intersection Observer API, it is to use 'FrozenArray' for thresholds.

WPT Test Case: https://wpt.fyi/results/intersection-observer/idlharness.window.html?label=experimental&label=master&aligned

Test Case Live Link: http://wpt.live/intersection-observer/idlharness.window.html

Failing Test -> IntersectionObserver interface: observer must inherit property "thresholds" with the proper type

___

Web-Spec: https://www.w3.org/TR/intersection-observer/#intersection-observer-interface

In Spec: readonly attribute FrozenArray<double> thresholds;

We have: readonly attribute sequence<double> thresholds; & (double or sequence<double>) threshold = 0.0;

___

Just wanted to raise, so we can fix this as well. Might be difficult for me right away to do so if someone else can do it, good to take it up.

Thanks!
Comment 1 Ahmad Saleem 2023-08-13 13:59:52 PDT
Web-Spec issue: https://github.com/w3c/IntersectionObserver/issues/114
Comment 2 Ahmad Saleem 2023-08-13 14:04:33 PDT
I get following error: 

/Users/ahmadsaleem/Documents/GitHub-Webkit-origin/Webkit/WebKitBuild/Release/DerivedSources/WebCore/JSIntersectionObserver.cpp:297:135: error: 
      no member named 'thresholds' in 'WebCore::IntersectionObserver'
  ...*thisObject.globalObject(), throwScope, impl.thresholds())));


by doing following changes:

> Source/WebCore/page/IntersectionObserver.idl

Change to: readonly attribute FrozenArray<double> thresholds;

and delete: (double or sequence<double>) threshold = 0.0;

> Source/WebCore/page/IntersectionObserver.h:

Delete: const Vector<double>& thresholds() const { return m_thresholds; }

> Source/WebCore/page/IntersectionObserver.cpp:

Line 425: Changing 'thresholds()' to 'm_thresholds'
Comment 3 Tim Nguyen (:ntim) 2023-08-13 14:18:45 PDT
(In reply to Ahmad Saleem from comment #2)
> I get following error: 
> 
> /Users/ahmadsaleem/Documents/GitHub-Webkit-origin/Webkit/WebKitBuild/Release/
> DerivedSources/WebCore/JSIntersectionObserver.cpp:297:135: error: 
>       no member named 'thresholds' in 'WebCore::IntersectionObserver'
>   ...*thisObject.globalObject(), throwScope, impl.thresholds())));
> 

I think this change is the source of your issue:

> > Source/WebCore/page/IntersectionObserver.h:
> 
> Delete: const Vector<double>& thresholds() const { return m_thresholds; }
Comment 4 Ahmad Saleem 2023-08-13 14:37:13 PDT
(In reply to Tim Nguyen (:ntim) from comment #3)
> (In reply to Ahmad Saleem from comment #2)
> > I get following error: 
> > 
> > /Users/ahmadsaleem/Documents/GitHub-Webkit-origin/Webkit/WebKitBuild/Release/
> > DerivedSources/WebCore/JSIntersectionObserver.cpp:297:135: error: 
> >       no member named 'thresholds' in 'WebCore::IntersectionObserver'
> >   ...*thisObject.globalObject(), throwScope, impl.thresholds())));
> > 
> 
> I think this change is the source of your issue:
> 
> > > Source/WebCore/page/IntersectionObserver.h:
> > 
> > Delete: const Vector<double>& thresholds() const { return m_thresholds; }

Indeed! Without doing this and just modifying IDL, I do progress the test. Should I do PR?
Comment 5 Ahmad Saleem 2023-08-13 14:57:51 PDT
PR - https://github.com/WebKit/WebKit/pull/16653
Comment 6 EWS 2023-08-13 17:15:07 PDT
Committed 266857@main (37667adc938e): <https://commits.webkit.org/266857@main>

Reviewed commits have been landed. Closing PR #16653 and removing active labels.
Comment 7 Radar WebKit Bug Importer 2023-08-13 17:16:13 PDT
<rdar://problem/113830183>