1193674 - If room context/name is unavailable, the title of the standalone is displayed as "{{roomName}} - Firefox Hello"

Status: RESOLVED FIXED

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

Description

[Mark Banner (:standard8)]

If room context is unavailable, then we're currently displaying the page title as "{{roomName}} - Firefox Hello".

This can be reproduced by taking a room url and dropping everything after the #.

Comment 1

[Manuel Casas Barrado [:mancas]]

Attachment: Bug 1193674 - If room context/name is unavailable, the title of the standalone is displayed as "{{roomName}} - Firefox Hello"

Comment 2

[Manuel Casas Barrado [:mancas]]

After an offline conversation with Pau, if there is no context available, the title of the standalone should be "Firefox Hello"

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8664688 [details] [diff] [review]
Bug 1193674 - If room context/name is unavailable, the title of the standalone is displayed as "{{roomName}} - Firefox Hello"

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

Just to note, I think the patch has the wrong commit message. Possibly a mistake when uploading it?

This patch also needs a test for the new code path. If you look in standaloneRoomViews_test.js, there's already a test for the title in the StandaloneRoomView describe, so you should be able to base it on that, but feel free to ping me if you need help.

::: browser/components/loop/standalone/content/js/standaloneRoomViews.jsx
@@ +401,5 @@
>            clientShortname: mozL10n.get("clientShortname2")
> +        });
> +
> +        if (!nextState.roomName && !this.state.roomName) {
> +          title = mozL10n.get("clientShortname2");

I wonder if it might be slightly nicer overall to do:

var roomName = nextState.roomName || this.state.roomName;

if (roomName) {
  this.setTitle(....);
} else {
  this.setTitle(mozL10n.get("clientShortname2");
}

That way we're not doing two calculations in the fallback case, and its clearer to read as to what the decision is based on.

Comment 4

[Manuel Casas Barrado [:mancas]]

Attachment: Bug 1193674 - If room context/name is unavailable, the title of the standalone is displayed as "{{roomName}} - Firefox Hello"

Comment 5

[Manuel Casas Barrado [:mancas]]

I've added the required tests to the patch Mark. Also I took into account your code style suggestions. Could you review it? :)

Thanks!

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 8665339 [details] [diff] [review]
Bug 1193674 - If room context/name is unavailable, the title of the standalone is displayed as "{{roomName}} - Firefox Hello"

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

Looks great, thanks.

Comment 7

[Carsten Book [:Tomcat]]

seems this failed to apply:

adding 1193674 to series file
renamed 1193674 -> Bug-1193674---If-room-contextname-is-unavailable-t.patch
applying Bug-1193674---If-room-contextname-is-unavailable-t.patch
patching file browser/components/loop/test/standalone/standaloneRoomViews_test.js
Hunk #1 FAILED at 144
1 out of 1 hunks FAILED -- saving rejects to file browser/components/loop/test/standalone/standaloneRoomViews_test.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and refresh Bug-1193674---If-room-contextname-is-unavailable-t.patch

Comment 8

[Manuel Casas Barrado [:mancas]]

Attachment: If room context/name is unavailable, the title of the standalone is displayed as "{{roomName}} - Firefox Hello"

Comment 9

[Manuel Casas Barrado [:mancas]]

Minor issue related to an existing variable name, it is solved now.

Thanks Carsten

Comment 10

[Pulsebot]

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

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/53fbb6d6f8d3