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

Custom Error msg for camera not available #31

Closed
JohnMcLear opened this issue Apr 2, 2020 · 19 comments
Closed

Custom Error msg for camera not available #31

JohnMcLear opened this issue Apr 2, 2020 · 19 comments

Comments

@JohnMcLear
Copy link
Member

No description provided.

@orblivion
Copy link
Contributor

If I understand this correctly, you want a setting (in etherpad-lite's settings.js) that lets you replace the existing hard-coded error message?

@orblivion
Copy link
Contributor

And if so, why only this message? (There are three "Sorry"s in that file.)

@JohnMcLear
Copy link
Member Author

If I deny camera access I currently get

"Sorry, you need to install SSL certificates for your Etherpad instance to use WebRTC";

And just freestyle the error msg, "No camera available, make sure your webcam is plugged in" or something

@orblivion
Copy link
Contributor

orblivion commented Apr 11, 2020

Ah, yes. That initial error message fix I did (#32) was incomplete. There are more kinds of errors and at some point I was gonna handle them, so I guess that's what this task is. I think what you're describing may be AbortError, I think I noticed it as well.

@orblivion
Copy link
Contributor

FYI I switched to this one for a "short break" from the videoTrack.stop security issue and found myself knee deep* in breaking changes in the webRTC spec that's made this take way longer than expected. Good news is that we should have much better error handling after this though. I'll PR soon.

* (So get this: until recently, NotAllowedError either meant SSL error, or user didn't give permission; exact same error. This is why we were testing for https, it was the only way of telling them apart. NOW, NotAllowedError only means user didn't give permission, and SSL error is handled by mediaDevices being absent altogether! Which means now we can't use "getUserMedia is missing" as a sign that the browser doesn't support WebRTC. I think I found another way though.)

@orblivion
Copy link
Contributor

Should we close this issue?

@JohnMcLear
Copy link
Member Author

@orblivion How can I close it? Is it possible to set a custom message for an error? If so, how?

@orblivion
Copy link
Contributor

Oops, I sort of got distracted by the requisite general fix for error messages (but also your first answer was a bit unclear).

Adding a customization should be quick. Does settings.json sound like a good place?

BTW I'm curious why this is the only error message you want to customize?

@JohnMcLear
Copy link
Member Author

Yea. Well I guess exposing customizing any string presented by i18n makes sense?

@orblivion
Copy link
Contributor

Sounds good. So the tasks shall be:

  • i18nize all of the a/v connection error messages
  • Let the user override anything in the i18n json files within settings.json
  • Add instructions for the preceding in README

@orblivion
Copy link
Contributor

(no longer such a quick task, but probably not large)

@JohnMcLear
Copy link
Member Author

Yeah consider the admin might only want to override the en_gb string too not the global value.

@orblivion
Copy link
Contributor

I'd like to add a test for this but I'm not seeing any existing examples of testing different values in settings.json. Tests seem to just use the current user's settings.json. Is there a path I'm not seeing or would I need to forge a new path?

@JohnMcLear
Copy link
Member Author

Is it passed in clientVars?

@orblivion
Copy link
Contributor

Didn't see it anywhere in tests. But I figured the right place was in the backend anyway. See PR forthcoming.

@orblivion
Copy link
Contributor

Still need #46

@orblivion
Copy link
Contributor

Now that we merged ether/etherpad-lite#4000, we can do custom overrides for l10n. However, that doesn't yet include ep_webrtc error messages. For that we need to merge #46. We may want to reopen this issue until it's done.

@JohnMcLear JohnMcLear reopened this May 19, 2020
@JohnMcLear
Copy link
Member Author

@orblivion closable?

@JohnMcLear JohnMcLear removed their assignment Jul 18, 2020
@orblivion
Copy link
Contributor

Yep!

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

2 participants