Closed
Bug 1175825
Opened 9 years ago
Closed 9 years ago
Fix text chat room name display when no room name is available (due to crypto failure) and fix a mozL10n warning on desktop
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox41 fixed)
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(1 file)
(deleted),
patch
|
mikedeboer
:
review+
|
Details | Diff | Splinter Review |
Currently, if there's no room name available, we get "Welcome to {{conversationName}}". Also, on desktop we're getting a warning about mozL10n being undefined when entering a room.
I've got a patch in progress to fix both of these.
Assignee | ||
Comment 1•9 years ago
|
||
Fix text chat room name display when no room name is available (due to crypto failure) and fix a mozL10n warning on Loop desktop.
This fixes the warning with a slightly larger patch - as there was an issue with the room name when information couldn't be decrypted/obtained.
I've moved the generation of the L10n wrapping of the room name to the view code, which seems better overall.
I've also put back in the warning logging that goes to the console when information can't be decoded (UX doesn't want it on the UI at the moment).
Attachment #8624072 -
Flags: review?(mdeboer)
Comment 2•9 years ago
|
||
Comment on attachment 8624072 [details] [diff] [review]
Applies on top of the patches in bug 1168829.
Review of attachment 8624072 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me! I do wonder a bit about the logging of errors to the console, but our handling of errors requires some TLC regardless.
::: browser/components/loop/standalone/content/js/standaloneRoomViews.jsx
@@ +281,5 @@
> + if (!this.state.roomInfoFailure && nextState.roomInfoFailure) {
> + if (nextState.roomInfoFailure === ROOM_INFO_FAILURES.WEB_CRYPTO_UNSUPPORTED) {
> + console.error(mozL10n.get("room_information_failure_unsupported_browser"));
> + } else {
> + console.error(mozL10n.get("room_information_failure_not_available"));
What's the use of logging this info to the console? Is there a bug on file to fix up our error handling a bit?
Attachment #8624072 -
Flags: review?(mdeboer) → review+
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Mike de Boer [:mikedeboer] from comment #2)
> ::: browser/components/loop/standalone/content/js/standaloneRoomViews.jsx
> @@ +281,5 @@
> > + if (!this.state.roomInfoFailure && nextState.roomInfoFailure) {
> > + if (nextState.roomInfoFailure === ROOM_INFO_FAILURES.WEB_CRYPTO_UNSUPPORTED) {
> > + console.error(mozL10n.get("room_information_failure_unsupported_browser"));
> > + } else {
> > + console.error(mozL10n.get("room_information_failure_not_available"));
>
> What's the use of logging this info to the console? Is there a bug on file
> to fix up our error handling a bit?
UX don't want to surface this - because it requires extra actions on the part of the user which are probably unnecessary. However, if we're getting complaints of things not working, I really do want it logged so that we have a chance of finding out what is going on.
I'll add a comment in the code.
Flags: firefox-backlog+
Comment 5•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•