1078345 - Change standalone link clicker UI landing page "Firefox WebRTC" to "Firefox Hello"

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect)

Comment 1

[Mark Banner (:standard8)]

This is being fixed by bug 1079430.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

These were not changed by bug 1079430.

Searching 1394 files for "Firefox WebRTC" (case sensitive)

d:\fx2\browser\components\loop\standalone\content\js\webapp.js:
  233          React.DOM.header({className: "standalone-header header-box container-box"}, 
  234            ConversationBranding(null), 
  235:           React.DOM.div({className: "loop-logo", title: "Firefox WebRTC! logo"}), 
  236            React.DOM.h3({className: "call-url"}, 
  237              conversationUrl

d:\fx2\browser\components\loop\standalone\content\js\webapp.jsx:
  233          <header className="standalone-header header-box container-box">
  234            <ConversationBranding />
  235:           <div className="loop-logo" title="Firefox WebRTC! logo"></div>
  236            <h3 className="call-url">
  237              {conversationUrl}

2 matches across 2 files

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

I noticed that some of the strings weren't being passed their string replacement arguments when fixing the Firefox WebRTC occurrences. These appear to have been overlooked from my patch in bug 1047133.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch (with generated JS file from JSX)

Noticed an error in the JS. Fixed the JSX and generated the JS file the right way.

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8502965 [details] [diff] [review]
Patch (with generated JS file from JSX)

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

rs=me

::: browser/components/loop/standalone/content/js/webapp.jsx
@@ +36,5 @@
>    var UnsupportedBrowserView = React.createClass({
>      render: function() {
>        var useLatestFF = mozL10n.get("use_latest_firefox", {
>          "firefoxBrandNameLink": React.renderComponentToStaticMarkup(
> +          <a target="_blank" href="https://www.mozilla.org/firefox/">{mozL10n.get("brandShortname")}</a>

It's weird to have the URL hard-coded while the string isn't. It seems like the URL should be pulled from a pref or string file too otherwise they won't match.

If you want to leave this to a follow-up I'm fine with that even though this patch is causing the possible future divergence.

@@ +232,5 @@
>        return (
>          <header className="standalone-header header-box container-box">
>            <ConversationBranding />
> +          <div className="loop-logo"
> +               title={mozL10n.get("client_alttext", 

Nit: trailing whitespace

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

We talked over IRC about the URLs in this file and moved them to the properties file. They can't be placed in prefs since this is standalone UI which isn't part of Firefox.

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

Comment 7

[Mark Banner (:standard8)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> We talked over IRC about the URLs in this file and moved them to the
> properties file. They can't be placed in prefs since this is standalone UI
> which isn't part of Firefox.

We have a configuration file that's available. Can they be placed in that instead, or is there a specific reason for L10n?

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2aca8cff92dd

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Mark Banner (:standard8) from comment #7)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> > We talked over IRC about the URLs in this file and moved them to the
> > properties file. They can't be placed in prefs since this is standalone UI
> > which isn't part of Firefox.
> 
> We have a configuration file that's available. Can they be placed in that
> instead, or is there a specific reason for L10n?

Yeah they can be placed there instead. I didn't notice loop.config before. I filed bug 1081189 to get this changed.