829355 - Trusted UI for identity needs a localized title

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

The Trusted UI should display a localized "Sign In" title for Persona.

We can add the string 'persona-signin' to locale files, with the persona- prefix there to prevent overlap with other applications that have a signin concept (like the system app).

Comment 1

[Maria Sandberg [:mushi]]

Hm, I'm confused. 

I believe we can only have one title for the entire flow where Persona sign in might be a piece or all of it (paid vs free app). Per bug 813811 that title will be the name of the initiating app, so there shouldn't be any need to localize?

Comment 2

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Attachment: gaia PR 7533

Axel, I'm new to L10n - am I doing this correctly? And will locales not yet supported all default back to this en-US file?  Thank you!

Comment 3

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

(In reply to Maria Sandberg [:mushi] from comment #1)
> Hm, I'm confused. 
> 
> I believe we can only have one title for the entire flow where Persona sign
> in might be a piece or all of it (paid vs free app). Per bug 813811 that
> title will be the name of the initiating app, so there shouldn't be any need
> to localize?

Hi, Maria.  Because the persona sign-in is a completely separate component from marketplace.  The Trusty UI needs a title when identity is requested by a website or app using persona.

The title for marketplace can be the name of the initiating app.  But even if it wanted to, there's no way for the marketplace app to tell the identity app what title to give itself.  They are two separate layers, one on top of the other, in the Trusty UI stack.

Comment 4

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Sorry, I see we're also having this discussion on the parent bug.  Let's keep the discussion about the interaction of Marketplace and Identity over there (bug 813811).

Comment 5

[Jason Smith [:jsmith]]

Comment on attachment 700759 [details]
gaia PR 7533

Kaze I know is at the work week and can probably look at this quickly and he knows how to review l10n patches.

Comment 6

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Jason, thank you

Kaze, thank you in advance for taking a look!

Comment 7

[Fabien Cazenave [:kaze]]

This looks OK, I just have a small doubt regarding the initialization process — see my comment on the PR.

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 700759 [details]
gaia PR 7533

With Kaze's comment, I guess this is OK.

The string is a dialog title, right? Then CamelCase is OK, otherwise it should probably be just "Sign in" in sentence case, according to https://etherpad.mozilla.org/gaia-copy-guidelines

Comment 9

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

(In reply to Fabien Cazenave [:kaze] from comment #7)
> This looks OK, I just have a small doubt regarding the initialization
> process — see my comment on the PR.

Thank you for that comment - I have updated the PR

Comment 10

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

(In reply to Axel Hecht [:Pike] from comment #8)
> Comment on attachment 700759 [details]
> gaia PR 7533
> 
> With Kaze's comment, I guess this is OK.

Thank you.  I've updated the PR to init as Kaze recommends.

> The string is a dialog title, right? Then CamelCase is OK, otherwise it
> should probably be just "Sign in" in sentence case, according to
> https://etherpad.mozilla.org/gaia-copy-guidelines

Yes, this is a window title; the camel-case is deliberate

Comment 11

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Comment on attachment 700759 [details]
gaia PR 7533

Thanks for your feedback, Axel and Kaze!

Comment 12

[Axel Hecht [:Pike]]

Comment on attachment 700759 [details]
gaia PR 7533

Forwarding the review request to kaze, I'm not a reviewer in the gaia module.

Comment 13

[Fabien Cazenave [:kaze]]

Comment on attachment 700759 [details]
gaia PR 7533

l10n again, we need this

[Approval Request Comment]
Bug caused by (feature/regressing bug #): ?
User impact if declined: untranslated title
Testing completed: manual
Risk to taking this patch (and alternatives if risky): none

Comment 14

[Axel Hecht [:Pike]]

This is bb- because it started off as a clone of bug 813811 ? I only realize that now.

Comment 15

[Jason Smith [:jsmith]]

(In reply to Axel Hecht [:Pike] from comment #14)
> This is bb- because it started off as a clone of bug 813811 ? I only realize
> that now.

I think it was originally basecamp- because although the title shown was technically incorrect, the user would understand the intention of what the dialog was showing.

However - re-reading this bug, I now get the sense that the title shown isn't getting translated, so that actually makes this blocker worthy.

Can someone confirm that the existing "IdentityFlow" and "PaymentFlow" wording isn't getting translated to pt-BR? If it isn't, I'll nom both associated bugs.

Comment 16

[Jason Smith [:jsmith]]

Actually I just read the patch - yeah this isn't getting translated. This definitely should have been nomed to block originally with that rationale.

Comment 17

[Jason Smith [:jsmith]]

I've modified bug 813811 to reflect the fact that the payment title also isn't getting translated, so that's been nomed as well.

Comment 18

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/f95e3ce220ad4b1e09076a26817ab62ea4bff106

Comment 20

[Fernando Jiménez Moreno [:ferjm]]

It seems that identity tests are broken https://bugzilla.mozilla.org/show_bug.cgi?id=813811#c45 :\
I've filed Bug 831264 as a follow-up bug to fix them.

Comment 21

[Jason Smith [:jsmith]]

Verified in English and pt-BR - I'm seeing a translated title and Sign In in English.

Comment 22

[Lukas Blakk [:lsblakk] use ?needinfo]

Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.