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
[presentation-api] update IDL tests for a receiving user agent #5029
[presentation-api] update IDL tests for a receiving user agent #5029
Conversation
Notifying @louaybassbouss, @tidoust, and @zqzhang. (Learn how reviewing works.) |
Firefox (nightly channel)Testing web-platform-tests at revision b9fd95e |
Chrome (unstable channel)Testing web-platform-tests at revision b9fd95e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the actual IDL still needs a couple of updates.
The overall testing/reporting mechanism looks great otherwise!
presentBtn.onclick = () => { | ||
presentBtn.disabled = true; | ||
const stash = new Stash(stashIds.toController, stashIds.toReceiver); | ||
const request = new PresentationRequest('support/idlharness_receiving-ua.html'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, the Cast Application ID 09F0075E
now targets:
https://w3c-test.org/presentation-api/receiving-ua/support/idlharness_receiving-ua.html
No need to change the URL parameter into an array with a Cast URL as a fallback for now though, given that Cast devices do not implement the receiving side.
@@ -24,14 +27,13 @@ | |||
<script id='idl' type="text/plain"> | |||
partial interface Navigator { | |||
[SameObject] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also add the SecureContext
flag that has just made it to the spec here, on the Presentation
and PresentationConnectionList
interfaces, as well as on the events interface?
This can be done in another pull request though (we'll need one to update controlling tests in any case).
@@ -104,17 +106,21 @@ | |||
</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not appear in the diff, but as far as I can tell, the IDL here still has PresentationConnectionClosedReason
instead of PresentationConnectionCloseReason
(without d
), and needs to be updated accordingly.
@@ -58,7 +60,8 @@ | |||
}; | |||
|
|||
interface PresentationConnection : EventTarget { | |||
readonly attribute DOMString? id; | |||
readonly attribute USVString? id; | |||
readonly attribute USVString? url; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no ?
in the spec for these attributes.
These tests are now available on w3c-test.org |
Thanks for review. I updated IDL descriptions in |
This PR modifies
receiving-ua/idlharness.html
.idlharness.html
are moved tosupport/idlharness_receiving-ua.html
.idlharness-manual.html
for a controlling side.