995436 - Use different sponsored panel text for Release and non-Release

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Ed Lee :Mardak]

Seems like we'll want the non-Release sponsored panel text to say something about testing while the Release text will mention sponsoring/supporting Mozilla.

clarkbw will provide the desired text.

To implement this, I believe the pattern is to insert strings into the appropriate branding properties file, so we'll need to initialize the panel with a string instead of relying on the dtd/entity:
http://mxr.mozilla.org/mozilla-central/source/browser/branding/
http://mxr.mozilla.org/mozilla-central/source/browser/branding/unofficial/locales/en-US/brand.properties
http://mxr.mozilla.org/mozilla-central/source/browser/components/migration/content/migration.js#251

Comment 1

[u279076]

Probably something we should test but it's not something we can do in this iteration. Until we have a "release" build to test (ie. Beta/RC) we won't be able to confirm this works as designed. We can revisit testing this at that time if desired. Tagging as [qa-] for this iteration though.

Please let me know if something more is needed from QA here.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Seems like this should have been split into "provide the text" and "implement the text differences" bug, since I don't think Bryan will be doing both.

Comment 3

[Bryan Clark (DevTools PM) [:clarkbw]]

Our initial experiment does not have actual sponsors, yet we want to test out the experience of having sponsors so we've enlisted a couple "Trial Sponsors".  Trial Sponsors may be interested in having Sponsored Tiles if we go down that path but a revenue agreement doesn't currently exist.

Trial Sponsor Text:
"This Trial Sponsor site was suggested because we hoped you’d find it interesting and because it supports Mozilla’s mission.  Learn more.."

A Sponsored version would drop the “Trial” word:
"This Sponsor site was suggested because we hoped you’d find it interesting and because it supports Mozilla’s mission.  Learn more.."

Comment 4

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to :Gavin Sharp (email [e-mail]) from comment #2)
> Seems like this should have been split into "provide the text" and
> "implement the text differences" bug, since I don't think Bryan will be
> doing both.

Indeed, string finalized. I believe maksik is taking over now.

Comment 5

[Ed Lee :Mardak]

Attachment: v1

Not exactly sure what the format should be for brand.properties in that should the key be "newtab." prefixed? All the other ones don't have "."s.

Comment 6

[Ed Lee :Mardak]

Actually, looking at this patch, I see there's nightly, aurora, unofficial, official.

clarkbw, should Beta have the Release text or not-Release text?

Comment 7

[Drew Willcoxon :adw]

Comment on attachment 8412322 [details] [diff] [review]
v1

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

It's not clear to me reading this bug that official branding maps to non-trial sponsors and all other branding maps to trial sponsors.  Bryan's comments here so far don't really make those connections.  But I might be missing context.

r+ assuming those are the right mappings.

(In reply to Ed Lee :Mardak from comment #5)
> Not exactly sure what the format should be for brand.properties in that
> should the key be "newtab." prefixed? All the other ones don't have "."s.

I'm not sure, either.  Maybe it's better not to use a newtab prefix in case we show these some other place in the future.  I don't think it's a big deal either way.

sponsoredPanelBrandMessage is a little vague.  You can imagine other sponsored things in the app.  I'd suggest sponsoredTileMessage or sponsoredSiteMessage.  Only suggestions, up to you.

Comment 8

[Ed Lee :Mardak]

Attachment: v2

maksik updated the patch to only show the non-Trial message for release-type channels, so that means Beta will show Trial.

Comment 9

[Drew Willcoxon :adw]

Comment on attachment 8413060 [details] [diff] [review]
v2

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

::: browser/base/content/newtab/page.js
@@ +24,5 @@
>      button.addEventListener("click", this, false);
>  
>      // Initialize sponsored panel
>      this._sponsoredPanel = document.getElementById("sponsored-panel");
> +    if (UpdateChannel.get().startsWith("release")) {

For posterity, I had a question about this part, but Gavin says:

> adw: the patch there does UpdateChannel.get().startsWith("release") to see if it's a "release-type" channel
> gavin: that shouldn't be based on the update channel
> gavin: #ifdef RELEASE_BUILD is probably better
> gavin: (that is also defined in beta builds)
> adw: Mardak says clarkbw said we want the "trial" language for betas too
> gavin: well I think I disagree with them
> ...
> gavin: we can sort it out later, the patch is OK temporarily
> gavin: (well, that part of the patch)

Comment 10

[Ed Lee :Mardak]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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