This bug tracks the completion of the work already started last week as part of the test in bug 1188455, to implement a new design for the about:privatebrowsing page that includes a section on Tracking Protection.

This is the updated design we're implementing: http://cl.ly/image/241a013L2Z3e And an updated mockup is here: https://invis.io/2K3PADF8N There is one specific change from the previous design that I'd like to call attention to, that is instead of having a "Privacy Preferences" link we have a "Turn off Tracking Protection" link. That's much better for users as we don't open the Preferences tab, since there is confusion when it's open in a private window as to why it says "Firefox will: Remember history". On the other hand the preference change is stored to disk when clicking the link, it does not only affect the current session. Is this an acceptable trade-off?

Aislinn, the mockup says that we're showing "See how this works" both when the feature is enabled and disabled. However, if I remember correctly the tour only works when the feature is enabled. Are you fine with removing the button on the disabled version? Separate issue, can we have the link to turn on Tracking Protection again not be inline with the sentence, but on a separate line? Having it inline causes issues with localization.

(In reply to :Paolo Amadini from comment #2) > Aislinn, the mockup says that we're showing "See how this works" both when > the feature is enabled and disabled. However, if I remember correctly the > tour only works when the feature is enabled. Are you fine with removing the > button on the disabled version? > > Separate issue, can we have the link to turn on Tracking Protection again > not be inline with the sentence, but on a separate line? Having it inline > causes issues with localization. Yes, please remove.

As to the turn on link, lets put it where the button was centered. I'll get an updated mock for you from Brian.

(In reply to :Paolo Amadini from comment #1) > This is the updated design we're implementing: > > http://cl.ly/image/241a013L2Z3e Looks nice! :-) > And an updated mockup is here: > > https://invis.io/2K3PADF8N > > There is one specific change from the previous design that I'd like to call > attention to, that is instead of having a "Privacy Preferences" link we have > a "Turn off Tracking Protection" link. That's much better for users as we > don't open the Preferences tab, since there is confusion when it's open in a > private window as to why it says "Firefox will: Remember history". On the > other hand the preference change is stored to disk when clicking the link, > it does not only affect the current session. Is this an acceptable trade-off? What does that preference capture? Is there any other UI that can change the same preference? If this is a preference that can only be changed from a private window, storing it on the disk will reveal whether the user has been using private browsing, which violates their privacy.

(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #5) > What does that preference capture? Is there any other UI that can change > the same preference? > > If this is a preference that can only be changed from a private window, > storing it on the disk will reveal whether the user has been using private > browsing, which violates their privacy. I believe it is the exact same as unticking the checkbox in about:preferences#privacy, which flips the privacy.trackingprotection.pbmode.enabled pref. This checkbox can be changed from the preferences page in normal mode as well. Leaving the ni? for Paolo to confirm.

What Brian said.

Bug 1190427 - Update the design of about:privatebrowsing. r=ttaubert

This replaces the existing about:privatebrowsing design completely. I reduced the version displayed in non-private windows to the bare essentials, reusing just two strings we already had for that case, as I don't think it will be often discovered. For the moment there is some self-contained code that disables the Tracking Protection section, designed to be easily removed in one go in bug 1175606. Tryserver build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0aac922a5e87

Aislinn, you asked for Helvetica Light for the headings, but I'm not sure what font this maps to exactly. Can you please attach an HTML file (as opposed to a screenshot) that includes the CSS I might use to obtain the desired typeface on Mac OS X?

(In reply to :Paolo Amadini from comment #14) > Aislinn, you asked for Helvetica Light for the headings, but I'm not sure > what font this maps to exactly. Can you please attach an HTML file (as > opposed to a screenshot) that includes the CSS I might use to obtain the > desired typeface on Mac OS X? can you use this? body { font-family: "HelveticaNeue-Light", "Helvetica Neue Light", "Helvetica Neue", Helvetica, Arial, "Lucida Grande", sans-serif; font-weight: 300; }

(In reply to agrigas from comment #15) > body { > font-family: "HelveticaNeue-Light", "Helvetica Neue Light", "Helvetica > Neue", Helvetica, Arial, "Lucida Grande", sans-serif; > font-weight: 300; > } For me, that renders exactly like in the screenshot.

(In reply to :Paolo Amadini from comment #16) > (In reply to agrigas from comment #15) > > body { > > font-family: "HelveticaNeue-Light", "Helvetica Neue Light", "Helvetica > > Neue", Helvetica, Arial, "Lucida Grande", sans-serif; > > font-weight: 300; > > } > > For me, that renders exactly like in the screenshot. ok well maybe you don't have that font installed on your machine?

Quite possible, so the question is whether we want to use fonts that may not be present on user machines?

(In reply to :Paolo Amadini from comment #18) > Quite possible, so the question is whether we want to use fonts that may not > be present on user machines? It will default to helvetica regular in which case that is fine. When user is on a mac - it should show up, when not, it will default to something sufficient.

https://reviewboard.mozilla.org/r/15113/#review13653 ::: browser/components/privatebrowsing/content/aboutPrivateBrowsing.js:36 (Diff revision 1) > - // Private browsing window, classic version. > - document.getElementById("learnMore").setAttribute("href", learnMoreURL); > + document.getElementById("favicon") > + .setAttribute("href", "chrome://global/skin/icons/question-16.png"); Maybe define this as a constant, e.g. const FAVICON_QUESTION = "...png"; ::: browser/components/privatebrowsing/content/aboutPrivateBrowsing.js:44 (Diff revision 1) > + document.getElementById("favicon") > + .setAttribute("href", "chrome://browser/skin/Privacy-16.png"); Should use a constant here too. ::: browser/components/privatebrowsing/content/aboutPrivateBrowsing.js:69 (Diff revision 1) > + document.getElementById("trackingProtectionSection") > + .setAttribute("style", "display: none;"); .setAttribute("hidden", "true"); ::: browser/components/privatebrowsing/content/aboutPrivateBrowsing.xhtml:34 (Diff revision 1) > - <div class="showPrivate"> > + <div id="bar" class="showPrivate">&privateWindow.title;</div> Couldn't we use a <h1> for this here? ::: browser/components/privatebrowsing/content/aboutPrivateBrowsing.xhtml:38 (Diff revision 1) > + <div class="sectionHeader">&aboutPrivateBrowsing.title;</div> <h2>? ::: browser/components/privatebrowsing/content/aboutPrivateBrowsing.xhtml:63 (Diff revision 1) > - <p class="showTpEnabled"><a id="startTour">&trackingProtection.startTour;</a></p> > + <div class="sectionHeader">&trackingProtection.title; <h2>? ::: browser/components/privatebrowsing/content/aboutPrivateBrowsing.xhtml:83 (Diff revision 1) > - accesskey="&trackingProtection.enable.accesskey;"/> > - </div> > + <p id="tpStartTour" > + class="showTpEnabled"><a id="startTour">&trackingProtection.startTour;</a></p> What's the a11y story here? Looks like I cant' focus it using the keyboard.

Comment on attachment 8643697 [details] MozReview Request: Bug 1190427 - Update the design of about:privatebrowsing. r=ttaubert Thanks RB.

(In reply to Tim Taubert [:ttaubert] from comment #20) > Couldn't we use a <h1> for this here? I tried that but I then was inheriting the common in-content styles like margins, colors and font sizes that I then had to reset in the CSS to match the result we wanted. It turned out it was much cleaner to use a generic block element containing the text (which is still perfectly valid HTML) than using the h1 and h2 tags. > What's the a11y story here? Looks like I can't focus it using the keyboard. Hm, it can be focused but there is no visual feedback. Will try to fix.

For the font issue, this clarified things a bit: https://en.wikipedia.org/wiki/List_of_typefaces_included_with_OS_X So, Helvetica does not have the Light variant while Helvetica Neue does. Using a font-family of "Helvetica Neue" and a font-weight of 300 makes the result very similar to the mockup.

Comment on attachment 8643697 [details] MozReview Request: Bug 1190427 - Update the design of about:privatebrowsing. r=ttaubert Bug 1190427 - Update the design of about:privatebrowsing. r=ttaubert

(In reply to :Paolo Amadini from comment #23) > For the font issue, this clarified things a bit: > > https://en.wikipedia.org/wiki/List_of_typefaces_included_with_OS_X > > So, Helvetica does not have the Light variant while Helvetica Neue does. > Using a font-family of "Helvetica Neue" and a font-weight of 300 makes the > result very similar to the mockup. Helvetica Neue is what was inteneded. So that works!

Here is the screenshot for the latest version of the patch. This should address all the nits, let me know if there is anything else.

NI'ing Bryan so he can verify it looks ok since its his design...

(In reply to :Paolo Amadini from comment #22) > (In reply to Tim Taubert [:ttaubert] from comment #20) > > Couldn't we use a <h1> for this here? > > I tried that but I then was inheriting the common in-content styles like > margins, colors and font sizes that I then had to reset in the CSS to match > the result we wanted. It turned out it was much cleaner to use a generic > block element containing the text (which is still perfectly valid HTML) than > using the h1 and h2 tags. Oh yeah no doubt it's valid HTML, semantically nicer (esp. for screen readers) would of course be to use <h1>/<h2>.

Comment on attachment 8643697 [details] MozReview Request: Bug 1190427 - Update the design of about:privatebrowsing. r=ttaubert https://reviewboard.mozilla.org/r/15115/#review13723 ::: browser/components/privatebrowsing/content/aboutPrivateBrowsing.css:140 (Diff revision 2) > -#tourFooter { > - margin: 16px auto; > + margin-left: auto; > + margin-right: auto; Maybe margin: 0 auto ?

NOTE: We could either incorporate the strings from bug 1192088 before landing or do it there as a follow-up.

(In reply to Tim Taubert [:ttaubert] from comment #29) > ::: browser/components/privatebrowsing/content/aboutPrivateBrowsing.css:140 > > + margin-left: auto; > > + margin-right: auto; > > Maybe margin: 0 auto ? Here I have to keep the default vertical margin of the P element, which is a browser-level value technically unknown to us (although it's probably 1.12em according to <http://www.w3.org/TR/CSS21/sample.html>).

(In reply to Tim Taubert [:ttaubert] from comment #30) > NOTE: We could either incorporate the strings from bug 1192088 before > landing or do it there as a follow-up. Let's land what we have and handle follow-ups there.

Why is this hardcoding fonts ? The common.css stylesheet already provides the right font per platform (Helvetica Neue on OSX, Segoe UI on Windows, Ubuntu on Linux, ...) using the message-box shorthand. Also, there's the info-pages.css stylesheet which can be used for an error-page like layout.

(In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment #35) > Why is this hardcoding fonts ? The common.css stylesheet already provides > the right font per platform (Helvetica Neue on OSX, Segoe UI on Windows, > Ubuntu on Linux, ...) using the message-box shorthand. Apparently message-box maps to Lucida Grande UI on OS X, while the designers asked for Helvetica Neue. I don't know if the current fallbacks for other platforms have been explicitly requested. The font specification in the comments above looks similar to others I found in the tree, so I guess this might be the desired result, although the designers may better answer this question. > Also, there's the info-pages.css stylesheet which can be used for an > error-page like layout. Thanks for the pointer, I didn't know this existed, can be useful for the future. I've taken a look and I think the layout we want here is distinct enough that we should just use the in-content style sheet as a starting point.

(In reply to :Paolo Amadini from comment #36) > (In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment > #35) > > Why is this hardcoding fonts ? The common.css stylesheet already provides > > the right font per platform (Helvetica Neue on OSX, Segoe UI on Windows, > > Ubuntu on Linux, ...) using the message-box shorthand. > > Apparently message-box maps to Lucida Grande UI on OS X, while the designers > asked for Helvetica Neue. I don't know if the current fallbacks for other > platforms have been explicitly requested. The font specification in the > comments above looks similar to others I found in the tree, so I guess this > might be the desired result, although the designers may better answer this > question. Lucida Grande is used for versions older than Yosemite, Yosemite uses Helvetica Neue (as message-box font). My guess is that designers aimed to be consistent with the current UI.

Why "Turn Tracking Protection On/Off" is presented as link and "See how this works" is presented as button? I'm asking because this looks quite jarring in Polish.

(In reply to Stefan Plewako [:stef] from comment #38) > Why "Turn Tracking Protection On/Off" is presented as link and "See how this > works" is presented as button? To give different emphasis to the two controls. > I'm asking because this looks quite jarring in Polish. What is exactly the difference from the English version and the Polish version?

(In reply to :Paolo Amadini from comment #39) > > I'm asking because this looks quite jarring in Polish. > > What is exactly the difference from the English version and the Polish > version? I don't know if Polish makes big difference here, roles seem reverted - buttons usually execute commands and hyperlinks reference documents.

Yeah, we're giving more weight to the visual hierarchy here (think of the de-emphasized "Cancel" links in some modern web UIs, which are not links but actual JavaScript commands). For technical users, we're making the action explicit by displaying the URL on hover for the button and nothing for the Turn On / Turn Off link.

Not sure why I'm NI'd?

(In reply to agrigas from comment #42) > Not sure why I'm NI'd? For comment 36.

(In reply to :Paolo Amadini from comment #36) > (In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment > #35) > > Why is this hardcoding fonts ? The common.css stylesheet already provides > > the right font per platform (Helvetica Neue on OSX, Segoe UI on Windows, > > Ubuntu on Linux, ...) using the message-box shorthand. > > Apparently message-box maps to Lucida Grande UI on OS X, while the designers > asked for Helvetica Neue. I don't know if the current fallbacks for other > platforms have been explicitly requested. The font specification in the > comments above looks similar to others I found in the tree, so I guess this > might be the desired result, although the designers may better answer this > question. > > > Also, there's the info-pages.css stylesheet which can be used for an > > error-page like layout. > > Thanks for the pointer, I didn't know this existed, can be useful for the > future. I've taken a look and I think the layout we want here is distinct > enough that we should just use the in-content style sheet as a starting > point. Paolo is right - we should adapt what we need to so that on Mac platforms using the latest OX QA can verify Helvetica Neue is being displayed.

(In reply to agrigas from comment #44) > Paolo is right - we should adapt what we need to so that on Mac platforms > using the latest OX QA can verify Helvetica Neue is being displayed. What about other platforms (Windows and Linux) ? On Windows, the page is the only page to use Arial, while the other pages use Segoe UI. On Linux, the same issue is happening, except other pages use Ubuntu instead of Segoe UI. Also, Helvetica Neue is only native on Yosemite. It looks out of place on both Mavericks (uses Lucida Grande) and El Capitan (uses San Francisco). Are you OK with using message-box ? It uses the native font per-platform, and is consistent with what other pages (in-content prefs, add-on manager, error-pages) use now. That includes using Helvetica Neue on OSX Yosemite.

(In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment #45) > (In reply to agrigas from comment #44) > > Paolo is right - we should adapt what we need to so that on Mac platforms > > using the latest OX QA can verify Helvetica Neue is being displayed. > > What about other platforms (Windows and Linux) ? On Windows, the page is the > only page to use Arial, while the other pages use Segoe UI. On Linux, the > same issue is happening, except other pages use Ubuntu instead of Segoe UI. > > Also, Helvetica Neue is only native on Yosemite. It looks out of place on > both Mavericks (uses Lucida Grande) and El Capitan (uses San Francisco). > > Are you OK with using message-box ? It uses the native font per-platform, > and is consistent with what other pages (in-content prefs, add-on manager, > error-pages) use now. That includes using Helvetica Neue on OSX Yosemite. I am ok with using the current default font for each version of OS for the platforms we support. For Mac, we should surface the specific fonts for Yosemite, Mavericks and El Capitain. Same for MS, we should reflect the default for each version. If the message-box does this - thats fine. I thought Paolo said it didn't handle the Mac default correctly.

Verified fixed FF 42.0a2 (2015-09-10) Win 7, Ubuntu 12.04, OS X 10.10.