Donation banner on about:tor page for 2016 campaign

added TorBrowserTeam201611R component::applications/tor browser crowdfunding parent::20413 priority::medium resolution::fixed severity::normal status::closed type::defect labels

Trac:
Parent: N/A to #20413 (moved)

Trac:
Cc: N/A to brade, mcs

I'm working on the banner, and it's getting there, but it still needs some polishing. In the meantime, here are the strings (for review) we are going to use for the banner text. (This text was suggested by Shari, with minor revisions.) If these strings look OK, I would like to get these added to the repository ASAP so we can give the translators a chance to do the translation ahead of our deadline for the next release.

https://github.com/arthuredelstein/torbutton/commit/20414+2

(For reference, I will post screenshots of the banner in the next comment.)

Trac:
Keywords: N/A deleted, crowdfunding, TorBrowserTeam201610R added
Status: new to needs_review
Cc: brade, mcs to brade, mcs, gk, ssteele, isabela

Trac:

(These four variants of the banner are rotated randomly.)

Replying to arthuredelstein:

I'm working on the banner, and it's getting there, but it still needs some polishing. In the meantime, here are the strings (for review) we are going to use for the banner text. (This text was suggested by Shari, with minor revisions.) If these strings look OK, I would like to get these added to the repository ASAP so we can give the translators a chance to do the translation ahead of our deadline for the next release.

https://github.com/arthuredelstein/torbutton/commit/20414+2

The strings look okay to me. I am old school, so I would capitalize Internet... but I suspect that is not something the cool kids do these days.

Replying to mcs:

Replying to arthuredelstein:

I'm working on the banner, and it's getting there, but it still needs some polishing. In the meantime, here are the strings (for review) we are going to use for the banner text. (This text was suggested by Shari, with minor revisions.) If these strings look OK, I would like to get these added to the repository ASAP so we can give the translators a chance to do the translation ahead of our deadline for the next release.

https://github.com/arthuredelstein/torbutton/commit/20414+2

The strings look okay to me. I am old school, so I would capitalize Internet... but I suspect that is not something the cool kids do these days.

At the risk of losing my Cool Kid status, I think I prefer it capitalized as well. Here's my new version:

https://github.com/arthuredelstein/torbutton/commit/20414+4

Here's the HTML/JS/CSS patch for review. For now it's only visible on US-English locales, but when we the strings have been translated, we can add another patch to show more locales.

https://github.com/arthuredelstein/torbutton/commits/20414+5 (two patches)

Trac:

After feedback from Shari, I've modified the text and layout somewhat. Please see images below.

The new patches are here: https://github.com/arthuredelstein/torbutton/commits/20414+8 The patch for strings alone is at https://github.com/arthuredelstein/torbutton/commit/9eed15d158f4763471150a97c83bb8d772774d57

I cherry-picked the strings (commit 269031f7aba449312189f73753e616fb82e5af48).

Kathy and I reviewed the JS/HTML/CSS changes. They look okay; we noticed just a few small things:

The comment says "2016 Dec 1" for the start date but the code uses October 1st.
Please remove some of the console.log() statements.
"fitt" should be "fit" in one comment.
The CSS has some rules for other browsers, e.g., -khtml-user-select: none (but if the same CSS file is going to be used on the website, maybe you do not want to remove these).
It would be nice to clean up the extensions.torbutton.donation_banner2016.shown_count pref somehow. Maybe reset it after the end date has been reached? This is not a big deal though; I do not know if we cleaned up last year's pref.

Replying to mcs:

Kathy and I reviewed the JS/HTML/CSS changes. They look okay; we noticed just a few small things:

Thanks for the review and catching all my mistakes!

The comment says "2016 Dec 1" for the start date but the code uses October 1st.

Fixed. The comment and code both now set a start date of November 15.

Please remove some of the console.log() statements.

Done.

"fitt" should be "fit" in one comment.

Fixed.

The CSS has some rules for other browsers, e.g., -khtml-user-select: none (but if the same CSS file is going to be used on the website, maybe you do not want to remove these).

Yes, I'll leave those in for that reason.

It would be nice to clean up the extensions.torbutton.donation_banner2016.shown_count pref somehow. Maybe reset it after the end date has been reached? This is not a big deal though; I do not know if we cleaned up last year's pref.

I've added some code to clean up the pref.

New version: https://github.com/arthuredelstein/torbutton/commit/20414+9

okay, I gave it a whirl and it looks good to me with some nits addressed:

What is the scope of count? It seems to me we could rewrite

    if (Services.prefs.prefHasUserValue(shownCountPref)) {
      count = Services.prefs.getIntPref(shownCountPref);
    } else {
      count = 0;
    }

into

    let count = 0;
    if (Services.prefs.prefHasUserValue(shownCountPref)) {
      count = Services.prefs.getIntPref(shownCountPref);
    }

s/that stop observing/that stops observing/
We put in the translations early to give translators time to produce the properly localized strings for non en-US Tor Browser versions. Yet the code is only concerned with en-US bundles anyway. What is the plan here then? Don't we want to ship 6.0.6 with the donation banner enabled for as many users as possible (this is more a needs_information point as I am not sure whether we should fix the code or not)?

Trac:
Status: needs_review to needs_revision
Keywords: TorBrowserTeam201610R deleted, TorBrowserTeam201611 added

Replying to gk:

okay, I gave it a whirl and it looks good to me with some nits addressed:

Thanks for the review.

What is the scope of count?

Fixed.

s/that stop observing/that stops observing/

Fixed.

We put in the translations early to give translators time to produce the properly localized strings for non en-US Tor Browser versions. Yet the code is only concerned with en-US bundles anyway. What is the plan here then? Don't we want to ship 6.0.6 with the donation banner enabled for as many users as possible (this is more a needs_information point as I am not sure whether we should fix the code or not)?

I have currently added several locales. I hope more can be translated in time for the release (including French and Russian). If so, we'll need to add those to the kBannerLocales list when they are ready.

New version: https://github.com/arthuredelstein/torbutton/commit/20414+11

Trac:
Status: needs_revision to needs_review

I applied .2 to master and maint-1.9.5 (commits c8ec31a111cf15971a341df02bca04fe09dd3bed and f5b70b5a4d5f5fd8db9acb74d62c3cc1b86f3ee2).

After updating the translations on master I realized that tr is ready as well and activates that one on maint-1.9.5 (6af7c834e9f79af974625d0f925d064a098bbee8) after getting the banner translations on that branch as well (94e5cfa96ed3ce4a3172efcbda8638e280de637e).

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

(I guess it can't hurt to test the code better before 6.0.6 is getting out, though ;) )

Trac:
Keywords: TorBrowserTeam201611 deleted, TorBrowserTeam201611R added

I copied Isabela's pt-BR translation of the banner strings over to the pt locale in Transifex; they are now available for import.

Here's another patch that adds the pt locale, delays launch by a week to November 22 (because the donation page needs a little more preparation time), and improves the font-size and appearance of the donate button.

https://github.com/arthuredelstein/torbutton/commit/20414+12

Trac:
Resolution: fixed to N/A
Status: closed to reopened

Alright. Looks good to me. Applied to master and maint-1.9.5 (commit 64f16026a2e9899c136e925f4487721d2746c56e and ba95ab383ce6ecdcdba9cc905095b0a85ad1b08e). I updated the pt translations on maint-1.9.5 with commit e6ce4a86360b4b17f690dcd229bb8b478fa19679.

Trac:
Status: reopened to closed
Resolution: N/A to fixed

Here's one more minor revision. If there isn't time to include this revision in the release, it's not really a problem. This patch simply delays the banner launch by one more day (to November 23) and also fixes a minor mistake in the code that doesn't stop it from functioning correctly.

https://github.com/arthuredelstein/torbutton/commit/20414+13

Trac:
Resolution: fixed to N/A
Status: closed to reopened

Trac:
Status: reopened to needs_review

Fixed with 2f8570e04f2465710b02abaf1e0cfd3059580134 (master) and 1a79b90c4cf41558d2b637b6314e0c64338d158d (maint-1.9.5).

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

I just discovered that the right arrow on the donate button that I just added is failing to render on OS X. Very sorry about this last minute discovery! Here's a patch to fix it:

https://github.com/arthuredelstein/torbutton/commit/20414+14

(I have tested this latest patch on all 3 platforms and it looks OK.)

Trac:
Status: closed to reopened
Resolution: fixed to N/A

Trac:
Status: reopened to needs_review

Applied to master and maint-1.9.5 (0a5af31aed37e4fd9faec408455b2d4ae139841e and e22642323f09efd18b4315d0b7145a781bb3dbdc). It won't make it into 6.5a4. We try to get it squeezed into 6.0.6, though.

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

closed

mentioned in issue #21131 (moved)

mentioned in issue #23483 (moved)

moved to tpo/applications/tor-browser#20414 (closed)

mentioned in issue tpo/applications/tor-browser#21131 (closed)

mentioned in issue tpo/applications/tor-browser#23483 (closed)

Donation banner on about:tor page for 2016 campaign

Child items ...

Activity