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.

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

binaryType setter requirements are wrong #2897

Closed
annevk opened this issue Aug 28, 2023 · 2 comments · Fixed by #2909
Closed

binaryType setter requirements are wrong #2897

annevk opened this issue Aug 28, 2023 · 2 comments · Fixed by #2909

Comments

@annevk
Copy link
Member

annevk commented Aug 28, 2023 •
edited
Loading

On setting, if the new value is either the string "blob" or the string "arraybuffer", then set the IDL attribute to this new value. Otherwise, throw a SyntaxError.

Due to BinaryType Web IDL will only ever hand you "blob" or "arraybuffer". All other values result in the binding layer returning early (not throwing).

I will correct that on WPT.

Also, it appears that Gecko and the specification disagree with WebKit on the default value. I will add coverage for "blob" on WPT, but I also think we should continue to discuss this over in #2170.

cc @youennf @jan-ivar
@annevk
Copy link
Member Author

annevk commented Aug 28, 2023 •
edited
Loading

Corrected tests: web-platform-tests/wpt#41663.

WebKit work: WebKit/WebKit#17121.

Also resolved Gecko's https://bugzilla.mozilla.org/show_bug.cgi?id=1728325 as INVALID and updated https://bugs.chromium.org/p/webrtc/issues/detail?id=2276.

annevk added a commit to web-platform-tests/wpt that referenced this issue Aug 28, 2023
@annevk
Correct RTCDataChannel's binaryType tests
e0a250f 
See w3c/webrtc-pc#2897 for context.
@annevk annevk mentioned this issue Aug 28, 2023
Merged
annevk added a commit to web-platform-tests/wpt that referenced this issue Aug 28, 2023
@annevk
Correct RTCDataChannel's binaryType tests
04227f0 
See w3c/webrtc-pc#2897 for context.
@fippo
Copy link
Contributor

fippo commented Aug 29, 2023

Chromium change just landed here, using IDL to validate things 💪
From how the change looks I bet this used to be a DOMString in the spec at some point in the distant past...
Still throws on blob but is not afraid of Jellyfish anymore.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 13, 2023
@annevk @moz-wptsync-bot
Bug 1850359 [wpt PR 41663] - Correct RTCDataChannel's binaryType test…
c5fdd28 
…s, a=testonly

Automatic update from web-platform-tests
Correct RTCDataChannel's binaryType tests

See w3c/webrtc-pc#2897 for context.
--

wpt-commits: 04227f01f6ef154ff9fb2476e713b71250ae4b2b
wpt-pr: 41663
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this issue Sep 14, 2023
@annevk @moz-wptsync-bot
Bug 1850359 [wpt PR 41663] - Correct RTCDataChannel's binaryType test…
2ecc79f 
…s, a=testonly

Automatic update from web-platform-tests
Correct RTCDataChannel's binaryType tests

See w3c/webrtc-pc#2897 for context.
--

wpt-commits: 04227f01f6ef154ff9fb2476e713b71250ae4b2b
wpt-pr: 41663
dontcallmedom added a commit that referenced this issue Nov 15, 2023
@dontcallmedom
Fix binaryType setter requirements
66a578f 
close #2897
@dontcallmedom dontcallmedom mentioned this issue Nov 15, 2023
Merged
Lightning00Blade pushed a commit to Lightning00Blade/wpt that referenced this issue Dec 11, 2023
@annevk @Lightning00Blade
Correct RTCDataChannel's binaryType tests
c7469a2 
See w3c/webrtc-pc#2897 for context.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix binaryType setter requirements w3c/webrtc-pc
2 participants
@fippo @annevk