1081189 - Move URLs out of loop.en-us.properties and into the config file

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect)

Description

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

Bug 1078345 added some URLs to the locales file but they should have been part of the Loop config file.

Comment 1

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

Attachment: Patch

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 8503350 [details] [diff] [review]
Patch

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

::: browser/components/loop/standalone/server.js
@@ +23,5 @@
>      "loop.config.feedbackProductName = '" + feedbackProductName + "';",
>      // XXX Update with the real marketplace url once the FxOS Loop app is
>      //     uploaded to the marketplace bug 1053424
>      "loop.config.marketplaceUrl = 'http://fake-market.herokuapp.com/iframe-install.html'",
> +    "loop.config.brandWebsiteUrl = 'https://www.mozilla.org/firefox/';",

These need to be added to the Makefile as well for make config. Unfortunately we've got them in two locations at the moment, and we've got a bug on file to fix that at some stage later.

Comment 3

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

Attachment: Patch v2

Comment 4

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Comment on attachment 8508949 [details] [diff] [review]
Patch v2

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

Sorry for the delay. This looks good, r=me with nits addressed.

::: 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={loop.config.brandWebsiteUrl}>{mozL10n.get("brandShortname")}</a>

Nit:

<a target="_blank" href={loop.config.brandWebsiteUrl}>
  {mozL10n.get("brandShortname")}
</a>

is a bit more readable.

@@ +86,1 @@
>                {mozL10n.get("get_firefox_button", {brandShortname: mozL10n.get("brandShortname")})}

Nit:

{mozL10n.get("get_firefox_button", {
  brandShortname: mozL10n.get("brandShortname")
})}

would be a bit more readable.

Comment 5

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

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/570579f42954

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5d0f7645977b

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8508949 [details] [diff] [review]
Patch v2

Approval Request Comment
Landed on aurora per IRC with lsblakk with a=loop-only