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

Checks in https://w3c.github.io/webrtc-pc/#dom-RTCDTMFSender-insertDTMF could be simplified #2254

Closed
youennf opened this issue Aug 5, 2019 · 3 comments
Assignees

Comments

@youennf
Copy link
Contributor

youennf commented Aug 5, 2019

Step 3 and 4 of insertDTMF are:
3. If transceiver's [[Stopped]] slot is true, throw an InvalidStateError.
4. If transceiver's [[CurrentDirection]] slot is recvonly or inactive, throw an InvalidStateError.
Step 6.6 (canInsertDTMF) will trigger an InvalidStateError if transceiver's [[CurrentDirection]] is neither "sendrecv" nor "sendonly".

Steps 4 is unneeded given step 6.6.
Since transceiver's [[CurrentDirection]] is supposed to be null in case transceiver's [[Stopped]] is true, we could also potentially remove step 3 as well.

It also seems checks 17.1 and 17.2 could be rewritten as a single check:
If transceiver's [[CurrentDirection]] slot is neither "sendrecv" nor "sendonly", abort these steps.

@alvestrand
Copy link
Contributor

Those seem like simplifications of the language with no change in behavior.
I suspect 17.1 is older than the decision to make transceiver.stop() clear CurrentDirection, I'm a bit hesitant to depend on the "remote action" here. Current language seems to do no harm.

@youennf
Copy link
Contributor Author

youennf commented Aug 30, 2019

Agreed this is editorial.
When reading the spec, it seemed odd to me that there could be a possibility for canInsertDTMF to return true and for insertDTMF to throw InvalidStateError.

@youennf
Copy link
Contributor Author

youennf commented Sep 26, 2019

Fixed with #2285

@youennf youennf closed this as completed Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants