Opened 9 months ago

Closed 8 months ago

#23483 closed defect (fixed)

Donation banner on about:tor page for 2017 campaign

Reported by: arthuredelstein Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: crowdfunding, TorBrowserTeam201709R, tbb-backported
Cc: gk, ssteele, akrieger@…, phoul Actual Points:
Parent ID: #23482 Points:
Reviewer: Sponsor:

Description

We want to add a new donation banner to the Tor Browser startup page for our fundraising campaign in 2017. The design is currently under development, and when it is ready, this ticket will be for patching torbutton to add the banner.

Our old banners were #20414 (2016) and #17565 (2015). We can hopefully reuse some of the code again.

Child Tickets

TicketStatusOwnerSummaryComponent
#23526closedtbb-teamAdd 2017 donation banner textApplications/Tor Browser

Attachments (3)

Screen Shot 2017-09-19 at 7.29.41 PM.png (117.6 KB) - added by arthuredelstein 9 months ago.
Screen Shot 2017-09-19 at 7.31.01 PM.png (109.7 KB) - added by arthuredelstein 9 months ago.
Screen Shot 2017-09-19 at 7.43.40 PM.png (122.5 KB) - added by arthuredelstein 9 months ago.

Download all attachments as: .zip

Change History (16)

Changed 9 months ago by arthuredelstein

Changed 9 months ago by arthuredelstein

Changed 9 months ago by arthuredelstein

comment:1 Changed 9 months ago by arthuredelstein

Here's a patch using the background image and layout concept from Mark Hardin and text from the crowfunding team. Please review. :)
https://github.com/arthuredelstein/torbutton/commit/23483

To test the banner, it's necessary to pull down the latest translations, build the torbutton xpi, install it in Tor Browser (release or alpha) and set the pref "extensions.torbutton.testBanner" to true in about:config.

Currently we have translations for [en, es, fr, it, tr] (plus some languages we don't currently release). If more translations have been completed at bundle time, then we will need to add those two-letter codes to the list located here.

I tested this patch on Mac, Linux, and Windows TBB 7.0. I also tested on Mac TBB 7.5. The banner is designed to first appear to users on October 23, 2017.

Here are some screen shots.



Last edited 9 months ago by arthuredelstein (previous) (diff)

comment:2 Changed 9 months ago by arthuredelstein

BTW, I ended up using design 2 among the two alternatives, because the text positioning in design 1 required too much finesse. Every time I got the text in the right location for one locale, it became badly positioned in another locale.

And I should mention, I wrote positioning code RTL languages. So it should work if we get Arabic, Farsi, or Hebrew translations. There aren't any translations for those locales yet, however, so I tested with dummy text.

comment:3 Changed 9 months ago by gk

Keywords: TorBrowserTeam201709R added; TorBrowserTeam201609 removed
Status: newneeds_review

comment:4 Changed 9 months ago by gk

Keywords: TorBrowserTeam201709 added; TorBrowserTeam201709R removed
Status: needs_reviewneeds_revision

Looks mostly good. Some nits:

1) bestSize and bestPadding are not declared in donation_banner.js. (seems I missed that last year ;) )
2)

+// Increase padding at right to "squeeze" text, until it gets
+// squeezed so much that it gets longer vertically.

is not correct as is in the case of RTL bundles the padding to the left increased. Could you reflect that in the comment?

In the commit message: "Bug 23483." -> "Bug 23483:"

comment:5 Changed 9 months ago by arthuredelstein

Keywords: TorBrowserTeam201709R added; TorBrowserTeam201709 removed
Status: needs_revisionneeds_review

Thanks for the review and good catches. I had used jshint but didn't realize that catching undefined variables was off by default.
https://github.com/arthuredelstein/torbutton/commit/23483+1

comment:6 Changed 9 months ago by gk

Keywords: TorBrowserTeam201709 added; TorBrowserTeam201709R removed
Status: needs_reviewneeds_revision

Another thing I found:

    if (bannerDataJSON.length > 0) {

it's not guaranteed that you have bannerDataJSON at all at this stage. Can you add a check for that before you query its length?

comment:7 Changed 9 months ago by arthuredelstein

Keywords: TorBrowserTeam201709R added; TorBrowserTeam201709 removed
Status: needs_revisionneeds_review

Another good find. Here is the new version:
https://github.com/arthuredelstein/torbutton/commit/23483+2

comment:8 Changed 9 months ago by gk

Keywords: tbb-backport added
Resolution: fixed
Status: needs_reviewclosed

Alright, looks good now, thanks. This landed on master (commit b3ff9863db338b2bd612f109e8bbce4c4af7cbd0).

comment:9 in reply to:  1 ; Changed 9 months ago by cypherpunks

Replying to arthuredelstein:

FWIW there's an error in the French translation: "la puissance de resistance numerique" should be ""la puissance de la resistance numerique.

comment:10 in reply to:  9 Changed 9 months ago by arthuredelstein

Cc: phoul added

Replying to cypherpunks:

Replying to arthuredelstein:

FWIW there's an error in the French translation: "la puissance de resistance numerique" should be ""la puissance de la resistance numerique.

Thanks. Colin, is that something you can edit on Transifex?

comment:11 Changed 8 months ago by gk

Keywords: tbb-backported added; tbb-backport removed

Backported for 7.0.7: commit 2e6d58145ff2de2ac705f2feebe3d594eab382a8 on maint-1.9.7.

comment:12 Changed 8 months ago by gk

Resolution: fixed
Status: closedreopened

comment:13 Changed 8 months ago by gk

Resolution: fixed
Status: reopenedclosed
Note: See TracTickets for help on using tickets.