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

Missing callbacks to setLocalDescription break FF nightly #73

Closed
fippo opened this issue Jun 16, 2014 · 5 comments
Closed

Missing callbacks to setLocalDescription break FF nightly #73

fippo opened this issue Jun 16, 2014 · 5 comments

Comments

@fippo
Copy link
Collaborator

fippo commented Jun 16, 2014

I just ran into an issue where FF nightly (33.0a1 2014-06-15) refused to work with the datachannel sample. It turned out that it failed because the success/error callbacks to setLocalDescription are missing (they're not optional per spec).
See e.g. https://github.com/GoogleChrome/webrtc/blob/master/samples/web/content/datachannel/js/main.js#L119

This seems to be used quite often, do a
grep -r setLocalDescription * | grep ')'
in the content directory.

I'll see if I can provide a patch.

@juberti
Copy link
Contributor

juberti commented Jun 16, 2014

OK. @marwahvikas can help.

@fippo
Copy link
Collaborator Author

fippo commented Jun 16, 2014

style-question before I start: is there a preferred way we can share the error callbacks among all demos? I tend to just pass console.error there.

I'll try to ignore that createAnswer is sometimes not called from the setRemoteDescription success callback for now. Another (style) issue.

Otherwise this seems straightforward.

@samdutton
Copy link
Contributor

is there a preferred way we can share the error callbacks among all demos? I tend to just pass console.error there.

Do you mean some error callback code to be used by all demos? Not at present. For the moment at least, I think it probably makes sense for individual demos to use their own handlers.

Whether to use the console or window.alert() (or both) depends on the context.

console.error is sensible for errors, though the existing demos use the plain old console.log. I've raised an issue to fix this: #130. On a tangent – the console API is quite powerful, and we should probably make more of it: https://developer.chrome.com/devtools/docs/console#using-the-console-api.

@fippo
Copy link
Collaborator Author

fippo commented Oct 7, 2014

@samdutton what I am worried about (long-term) is putting stuff in adapter.js that isn't shimming browser differences.

(see also the discussion in #74)

@fippo
Copy link
Collaborator Author

fippo commented Mar 16, 2015

nightly no longer insists since this is not possible to detect with the promise-based versions.

@fippo fippo closed this as completed Mar 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants