1184188 - The in-content preferences tracking checkbox labels and Learn More links run together

Status: VERIFIED FIXED

(Firefox :: General, defect, P1)

Description

[Stefan Plewako [:stef]]

Follow up to bug 1175920, see comment 24, 26 and https://bugzilla.mozilla.org/attachment.cgi?id=8634011

dntTrackingNotOkay.label2, trackingProtection.label and trackingProtectionPBM.label in browser/locales/en-US/chrome/browser/preferences/privacy.dtd need periods at the end, labels identifiers should be updated probably too

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Why only the tracking protection labels and not the rest of the checkbox labels within the preferences? This will create some internal inconsistency here.

Comment 2

[Stefan Plewako [:stef]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> Why only the tracking protection labels and not the rest of the checkbox
> labels within the preferences? This will create some internal inconsistency
> here.

Because only mentioned labels have "Learn more" links added at the end via styling, see attachment 8634011 [details].

Comment 3

[Francesco Lodolo [:flod]]

To be absolutely honest my choice would be to move further away those links, so it's clear that they're not part of the string and we don't need to fix existing translations/strings.

Comment 4

[Stefan Plewako [:stef]]

Attachment: fix

Patch adding periods, as discussed in bug 1175920, excluded from original patch based on incorrect assumption (they are not at the end of checkbox labels anymore, learn more links are).

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/56d2e2d60a60

Comment 6

[Justin Dolske [:Dolske]]

Hmm, I don't think this is the right fix.

Preference descriptions shouldn't be sentences. This is a pretty standard UI convention, and all other prefs in Firefox follow that convention.

It's a real problem that the "Learn More" links are too close (I've noticed this too and meant to file a bug). But I think a better fix would be to have a single "Learn More" link for the whole section, instead of placing one after each of the 3 items... Either as just a simple "Learn More" label at the bottom, or a more verbose "_Learn More_ about tracking protection and how Firefox keeps you safe." / "Learn more about _tracking protection_ and _DNT_." kind of thing.

We could just try fixing this with formatting (increase space between label/link, and make make the link text smaller). That might be enough for other cases we want to add a Learn More link, but having a bunch close together is going to be weird no matter how it's formatted.

NI phlsa for a UX decision.

Comment 7

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

(In reply to Justin Dolske [:Dolske] from comment #6)
> Hmm, I don't think this is the right fix.
> 
> Preference descriptions shouldn't be sentences. This is a pretty standard UI
> convention, and all other prefs in Firefox follow that convention.
> 
> It's a real problem that the "Learn More" links are too close (I've noticed
> this too and meant to file a bug). But I think a better fix would be to have
> a single "Learn More" link for the whole section, instead of placing one
> after each of the 3 items... Either as just a simple "Learn More" label at
> the bottom, or a more verbose "_Learn More_ about tracking protection and
> how Firefox keeps you safe." / "Learn more about _tracking protection_ and
> _DNT_." kind of thing.
> 
> We could just try fixing this with formatting (increase space between
> label/link, and make make the link text smaller). That might be enough for
> other cases we want to add a Learn More link, but having a bunch close
> together is going to be weird no matter how it's formatted.
> 
> NI phlsa for a UX decision.

Forwarding to Ash who has more context here.

FWIW, I'd be fine with having just one learn more link for the entire section.

I think the reason for adding this had something to do with localization, but I'm blanking on what the exact issue was.

Comment 8

[Ash Grigas]

(In reply to Philipp Sackl [:phlsa] please use needinfo from comment #7)
> (In reply to Justin Dolske [:Dolske] from comment #6)
> > Hmm, I don't think this is the right fix.
> > 
> > Preference descriptions shouldn't be sentences. This is a pretty standard UI
> > convention, and all other prefs in Firefox follow that convention.
> > 
> > It's a real problem that the "Learn More" links are too close (I've noticed
> > this too and meant to file a bug). But I think a better fix would be to have
> > a single "Learn More" link for the whole section, instead of placing one
> > after each of the 3 items... Either as just a simple "Learn More" label at
> > the bottom, or a more verbose "_Learn More_ about tracking protection and
> > how Firefox keeps you safe." / "Learn more about _tracking protection_ and
> > _DNT_." kind of thing.
> > 
> > We could just try fixing this with formatting (increase space between
> > label/link, and make make the link text smaller). That might be enough for
> > other cases we want to add a Learn More link, but having a bunch close
> > together is going to be weird no matter how it's formatted.
> > 
> > NI phlsa for a UX decision.
> 
> Forwarding to Ash who has more context here.
> 
> FWIW, I'd be fine with having just one learn more link for the entire
> section.
> 
> I think the reason for adding this had something to do with localization,
> but I'm blanking on what the exact issue was.

Philipp - when we reviewed this, you translated it into german and that's where it gets wierd to have no period between the pref label and the link since it all blends together. For english locales, no period is needed, I believed we were adding it in for international locales. If that distinction is too much overhead, than having periods to better differentiate between the label and the link - was the rationale for this change.

Agree we need more space between the label and link - it looks like its less than a full space now and if it is a full space - we should add a few more to give it some padding.

For now, we cannot create a unified FAQ page for both DNT and TP which is why we need both links. For most users, they will only see 2 prefs - 3 are only shown to users that through about:config enabled TP in regular mode (very small group). I don't think 2 learn more links necessitates a new formatting structure for this one instance of Tracking settings without thinking more holistically if our current treatment for Learn More across prefs should be re-considered...

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/56d2e2d60a60

Comment 10

[Marco Mucci [:MarcoM]]

Hi Matt, do you believe this bug requires QE verification?

Comment 11

[Matt Wobensmith [:mwobensmith][:matt:]]

Sure. Added myself for this.

Comment 12

[Matt Wobensmith [:mwobensmith][:matt:]]

Verified Fx42.0a1, 2015-07-22.

Comment 13

[Justin Dolske [:Dolske]]

Attachment: Screenshot on Nightly

So, this still looks bad on Nightly.

(In reply to agrigas from comment #8)

> Philipp - when we reviewed this, you translated it into german and that's
> where it gets wierd to have no period between the pref label and the link
> since it all blends together. For english locales, no period is needed, I
> believed we were adding it in for international locales. If that distinction
> is too much overhead, than having periods to better differentiate between
> the label and the link - was the rationale for this change.

Each locale has its own string, so there's no need to add it to English. (But I'd suggest unless there's a compelling reason to add the period, they should follow existing convention.)

> Agree we need more space between the label and link - it looks like its less
> than a full space now and if it is a full space - we should add a few more
> to give it some padding.

I've written a patch to do so.

Comment 14

[Justin Dolske [:Dolske]]

Attachment: Screenshot with spacing

Screenshot with spacing instead.

Comment 15

[Justin Dolske [:Dolske]]

Attachment: Patch (for spacing)

Trivial change, might as well review in parallel with the ui-review.

Comment 16

[Justin Dolske [:Dolske]]

(Note that bug 1183240 landed after this one and changed the strings again, so I just fixed the issue instead of attempting an actual backout + new fix.)

Comment 17

[Stefan Plewako [:stef]]

(In reply to Justin Dolske [:Dolske] from comment #15)
> Created attachment 8640193 [details] [diff] [review]
> Patch (for spacing)
> 
> Trivial change, might as well review in parallel with the ui-review.

With such spacing it will look bad in Polish (with or without period), exactly as in about:addons with "More" links.

Comment 18

[Justin Dolske [:Dolske]]

(In reply to Stefan Plewako [:stef] from comment #17)

> With such spacing it will look bad in Polish (with or without period),
> exactly as in about:addons with "More" links.

What, exactly, makes it "look bad"?

Comment 19

[Stefan Plewako [:stef]]

(In reply to Justin Dolske [:Dolske] from comment #18)
> What, exactly, makes it "look bad"?

1.5em of margin, Polish doesn't really have a concept of separating sentences by random amount of space and it looks like one line as opposed to link in a new line (as in version before all the changes) or something aligned to the opposite side (button or imperfect but still better links in data choices in advanced) or help button.

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8640193 [details] [diff] [review]
Patch (for spacing)

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

Kinda weird to be doing this patch in a VERIFIED-FIXED bug, especially since this patch is basically a mutated backout of the patch also in this bug. But code-wise, I'm fine with it.

I really wish we didn't use "Learn More". It's generic and doesn't really provide any differentiation between the three links, except that *maybe* they lead to different places. With the preferences being a XUL page, we don't show the URL of the link in the status-bar overlay, so users really have no idea what they're about to see even if they know how the browser works.

I would prefer that we went with "What is Do Not Track?" and "What is Tracking Protection?". And further, do we need a link on the non-private Tracking Protection row, considering it probably links to the same place as the private-browser tracking protection link?

Comment 21

[Justin Dolske [:Dolske]]

(In reply to Stefan Plewako [:stef] from comment #19)
> (In reply to Justin Dolske [:Dolske] from comment #18)
> > What, exactly, makes it "look bad"?
> 
> 1.5em of margin, Polish doesn't really have a concept of separating
> sentences by random amount of space and it looks like one line as opposed to
> link in a new line (as in version before all the changes) or something
> aligned to the opposite side (button or imperfect but still better links in
> data choices in advanced) or help button.

This seems like more of a stylistic objection than anything specifically regarding L10N limitations.

I don't think this UI is perfect, but adding space is an improvement. And given that other UI has also already been doing this, it's no worse than the status quo. UX may wish to consider a redesign, but that's fodder for a different bug.

Comment 22

[Justin Dolske [:Dolske]]

Reopening since I'm effectively backing out the original patch as being incorrect. I'm going to go ahead and land the updated patch since per the above, and since it's consistent with what comment 8 was expecting.

Comment 23

[Justin Dolske [:Dolske]]

url:        https://hg.mozilla.org/integration/fx-team/rev/1b022f0cb2883fb54ee3db4986a40ce31cf5b13c
changeset:  1b022f0cb2883fb54ee3db4986a40ce31cf5b13c
user:       Justin Dolske <[e-mail]>
date:       Fri Jul 31 18:39:56 2015 -0700
description:
Bug 1184188 - Add spacing before "Learn More" links in Privacy prefs. r=jaws

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1b022f0cb288

Comment 25

[Matt Wobensmith [:mwobensmith][:matt:]]

Verified latest fix, Nightly 42.0a1, 2015-08-05.