1147609 - Loop standalone should be able to handle the room name embedded in context or as a separate value

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Mark Banner (:standard8)]

Splitting this out from bug 1142522 as this is likely to be ready to land sooner.

The aim here is to make it so that the standalone UI can deal with either room name being part of the encrypted context, or it being the existing standalone parameter when retrieved from the server.

We'll also deal with displaying errors for edge cases when decryption fails.

Comment 1

[Mark Banner (:standard8)]

Attachment: WIP Make Loop's standalone UI work with roomName as an unecrypted parameter or as an encrypted part of context.

Current WIP, attaching here so I can read it through again and compare with tests more easily ;-)

Comment 2

[Mark Banner (:standard8)]

Sevaan: Matej: I could do with some recommendations for the failure case strings. Here's what I have currently:

"Room Information is unavailable. Please request a refresh link from the person you received this from."

This error is displayed when the room context (e.g. name, description, url) is unavailable. This could be because decryption has failed (missing/invalid key) or maybe for some strange reason, there's no data available.

"Sorry, your browser is unable to access the room information. Please ensure it is upgraded to the latest version."

This error is displayed when we can't decrypt the room context because the browser doesn't support the facilities that we need.

Comment 3

[Matej Novak [:matej]]

(In reply to Mark Banner (:standard8) from comment #2)
> Sevaan: Matej: I could do with some recommendations for the failure case
> strings. Here's what I have currently:
> 
> "Room Information is unavailable. Please request a refresh link from the
> person you received this from."

"No information about this conversation is available. Please request a new link from the person who sent it to you."

> This error is displayed when the room context (e.g. name, description, url)
> is unavailable. This could be because decryption has failed (missing/invalid
> key) or maybe for some strange reason, there's no data available.
> 
> "Sorry, your browser is unable to access the room information. Please ensure
> it is upgraded to the latest version."

"Your browser cannot access any information about this conversation. Please make sure you're using the latest version.

> This error is displayed when we can't decrypt the room context because the
> browser doesn't support the facilities that we need.

In both cases I think we should stick to "conversation" to be consistent. Does that work?

Comment 4

[Sevaan Franks [:sevaan]]

Those read great, Matej.

Comment 5

[Mark Banner (:standard8)]

Attachment: Make Loop's standalone UI work with roomName as an unecrypted parameter or as an encrypted part of context.

This depends on the patch in bug 1114563.

The basic idea here is for standalone to be able to decrypt the context, but fall back to the roomName if no context is supplied.

I've also created a new view for the standalone for the room name or failure to decrypt messages, but I'm also envisioning that it can be used for the rest of the context information once we've got the browser side hooked up.

I'll attach another patch in a moment that's not intended for landing, but makes it possible to create a room with encrypted context for testing purposes.

Comment 6

[Mark Banner (:standard8)]

Attachment: Hack to create a room with the name encrypted in the context field (not for committing)

Once applied and built, select start a conversation. You then get the room created (the title shows as "Encrypted" in the desktop as a workaround).

Then:

1) Copy the room link & paste into a browser.
2) Go back to the room generator, look up the loop.context.key pref and copy its value
3) Back on the url, add a hash, then append the copied key.

The room name should be shown (it'll be "Conversation 1" or something like that).

This is just a PoC hack and not intended for landing.

Comment 7

[Mike de Boer [:mikedeboer]]

Mark, can we pair-review this one? I'll make a general pass (nits, etc) today.

Comment 8

[Mike de Boer [:mikedeboer]]

Comment on attachment 8583739 [details] [diff] [review]
Make Loop's standalone UI work with roomName as an unecrypted parameter or as an encrypted part of context.

Review of attachment 8583739 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, forget about my pair-review request, this was more straightforward than I thought before.

So, apology for the delay!

r=me with comments addressed.

::: browser/components/loop/content/shared/js/activeRoomStore.js
@@ +228,5 @@
> +        }
> +
> +        var roomInfoData = new sharedActions.UpdateRoomInfo({
> +            roomOwner: result.roomOwner,
> +            roomUrl: result.roomUrl

nit: whoops! Indentation.

@@ +237,5 @@
> +          this.dispatcher.dispatch(roomInfoData);
> +          return;
> +        }
> +
> +        if (result.roomName && !result.context) {

Please add a comment for this case of 'legacy', non-encrypted room names.

@@ +266,5 @@
> +
> +          this.dispatcher.dispatch(roomInfoData);
> +        }.bind(this), function(err) {
> +          roomInfoData.roomInfoFailure = ROOM_INFO_FAILURES.DECRYPT_FAILED;
> +          this.dispatcher.dispatch(roomInfoData);

If you just alias `this.dispatcher` to `var dispatcher`, you won't need to do the bind() dance all the time.

::: browser/components/loop/standalone/content/l10n/en-US/loop.properties
@@ +127,5 @@
>  rooms_display_name_guest=Guest
>  rooms_unavailable_notification_message=Sorry, you cannot join this conversation. The link may be expired or invalid.
>  rooms_media_denied_message=We could not get access to your microphone or camera. Please reload the page to try again.
> +room_information_failure_not_available=No information about this conversation is available. Please request a new link from the person who sent it to you.
> +room_information_failure_unsupported_browser=Your browser cannot access any information about this conversation. Please make sure you're using the latest version.

O noes, strings! What shall we do with these?

::: browser/components/loop/test/standalone/standaloneRoomViews_test.js
@@ +40,5 @@
>    });
>  
> +  describe("StandaloneRoomContextView", function() {
> +    beforeEach(function() {
> +      sandbox.stub(navigator.mozL10n, "get").returnsArg(0);

Nice! I'll be using this from here on out. Sinon rocks.

Comment 9

[Mark Banner (:standard8)]

Re strings - they are for standalone so don't need uplifting.

Comment 10

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/fx-team/rev/c434080d10fe

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c434080d10fe