Telefonica logo should only appear on FTU with the "Get started" button in the panel

RESOLVED FIXED in mozilla37

Status

P2
normal
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: RT, Assigned: rgauthier)

Tracking

unspecified
mozilla37
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

4 years ago
Per https://bugzilla.mozilla.org/show_bug.cgi?id=1083466#c1 the Telefonica logo should be only displayed on FTU. 
It currently follows the same rules as the ToS and privacy policy links.
Later today, I'll be moving Fx36 bugs to follow the new priority definitions later today:

P1 - We would block shipment for this bug or turn off the feature if we can't fix it
P2 - We need/very much want this bug, but we wouldn't block shipment or turn off the feature.
P3 - Normal bug
P4, P5 are even lower priority

Right now I'm marking this a P1 (to align with the definitions of other Fx36 bugs), but later today this will move to P2.  This doesn't mean it has truly fallen in priorities.  We just want to save the "P1" category for a real "stop ship".
backlog: --- → Fx36+
Priority: -- → P1
Moving all P1->P2.  (P2 means a major bug that we very much want to fix, but we wouldn't stop ship or block the release for it.)
Here's the actual change from P1->P2 (per the previous comment).  P2 indicates a major bug, but not a stop ship.
Priority: P1 → P2
(Assignee)

Updated

4 years ago
Assignee: nobody → rgauthier
(Assignee)

Comment 4

4 years ago
Created attachment 8544056 [details] [diff] [review]
Show the telefonica logo only on first use
Attachment #8544056 - Flags: review?(nperriault)
Comment on attachment 8544056 [details] [diff] [review]
Show the telefonica logo only on first use

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

Looks good, r=me with comments addressed.

::: browser/app/profile/firefox.js
@@ +1669,5 @@
>  
>  pref("loop.enabled", true);
>  pref("loop.server", "https://loop.services.mozilla.com/v0");
>  pref("loop.seenToS", "unseen");
> +pref("loop.showTelefonica", true);

How about something more generic/brand agnostic, like showPartnerLogo?

::: browser/components/loop/content/js/panel.jsx
@@ +206,4 @@
>        };
>      },
>  
> +    renderTelefonica: function() {

How about renderPartnerLogo?

@@ +211,5 @@
> +        return null;
> +      }
> +
> +      var locale = mozL10n.getLanguage();
> +      navigator.mozLoop.setLoopPref('showTelefonica', false);

Note: I wish we had a Flux store for prefs, so we could dispatch an action from here; though creating a store for this bug seems overkill, so it's probably fine that way for now.

Maybe we should just file a new bug about this idea, and reference it here in a comment (if you agree it's a good idea).

::: browser/components/loop/test/desktop-local/panel_test.js
@@ +1038,5 @@
> +
> +         try {
> +           TestUtils.findRenderedDOMComponentWithClass(view, "powered-by");
> +         } catch (err) {
> +           done();

I'm confused, why do we need to use done() at all here? I can't see what's async. If it's just matter of ensuring the element isn't present, just use expect(view.getDOMNode().querySelector(".powered-by")).eql(null) as we've been doing a lot in similar situations.
Attachment #8544056 - Flags: review?(nperriault) → review+

Comment 6

4 years ago
OK letting ride train in 37 - so not uplifting to 36.  Great to land before aurora though for 37
backlog: Fx36+ → Fx37+
(Assignee)

Comment 7

4 years ago
Created attachment 8544646 [details] [diff] [review]
Show the telefonica logo only on first use

Concerning Nico's proposal to have a store to set prefs, I opened Bug 1118303
Attachment #8544646 - Flags: review?(nperriault)
(In reply to Romain Gauthier [:tOkeshu] from comment #7)
> Created attachment 8544646 [details] [diff] [review]
> Show the telefonica logo only on first use

This patch bitrots, could you please rebase it against latest fx-team? Thanks!
(Assignee)

Comment 9

4 years ago
Created attachment 8545238 [details] [diff] [review]
Show the telefonica logo only on first use
Attachment #8544056 - Attachment is obsolete: true
Attachment #8544646 - Attachment is obsolete: true
Attachment #8544646 - Flags: review?(nperriault)
Attachment #8545238 - Flags: review?(nperriault)
Comment on attachment 8545238 [details] [diff] [review]
Show the telefonica logo only on first use

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

Please add the compiled version of panel.jsx (ensure you've upgraded the react-tools npm package to 0.12.2)
Attachment #8545238 - Flags: review?(nperriault) → review-
(Assignee)

Comment 11

4 years ago
Created attachment 8545258 [details] [diff] [review]
Show the telefonica logo only on first use
Attachment #8545238 - Attachment is obsolete: true
Attachment #8545258 - Flags: review?(nperriault)
(Assignee)

Comment 12

4 years ago
Created attachment 8545268 [details] [diff] [review]
Show the telefonica logo only on first use
Attachment #8545258 - Attachment is obsolete: true
Attachment #8545258 - Flags: review?(nperriault)
Attachment #8545268 - Flags: review?(nperriault)
Comment on attachment 8545268 [details] [diff] [review]
Show the telefonica logo only on first use

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

Works great.
Attachment #8545268 - Flags: review?(nperriault) → review+
https://hg.mozilla.org/mozilla-central/rev/e8a928acfdab
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Iteration: 37.2 → 37.3 - 12 Jan
You need to log in before you can comment on or make changes to this bug.