818065 - PBM - Make an about:privatebrowsing page

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Ian Barlow (:ibarlow)]

When a user creates a new tab, they should land on a page that explains what private browsing is, and what it does / doesn't protect against. 

Designs to follow shortly. 

--

Note: this should be implemented before we consider Android Private Browsing v1 to be complete.

Comment 1

[Brian Nicholson (:bnicholson)]

Attachment: Implement about:privatebrowsing

Here's a preliminary patch that uses a heavily modified version of desktop's about:privatebrowsing page.

Comment 2

[Brian Nicholson (:bnicholson)]

Attachment: screenshot

Comment 4

[Brian Nicholson (:bnicholson)]

We'll need this to ship private browsing.

Comment 5

[Alex Keybl [:akeybl]]

(In reply to Brian Nicholson (:bnicholson) from comment #4)
> We'll need this to ship private browsing.

This really needs to land no later than early next week (given l10n impact). I suggest we land what we've got before the end of this week, and iterate on the UX afterwards. The string seems to be a sure thing, since it's been hardly changed from desktop's about:privatebrowsing.

Comment 6

[Ian Barlow (:ibarlow)]

Attachment: about:privatebrowsing UI mockup

Hi Brian, here is a mockup you can follow for this page. I'm not sure how you are building this, so can you let me know if you need mdpi/hdpi/xhdpi assets for the mask image?

Comment 7

[Brian Nicholson (:bnicholson)]

Attachment: screenshot (non-private)

Screenshot of current patch if user goes to about:privatebrowsing in a non-private tab.

Comment 8

[Ian Barlow (:ibarlow)]

Attachment: mockup

Let's at least style it to look like a light version of the real about:privatebrowsing. 

- Text colour is #222222, sizes are the same as the mockups above
- No mask image
- Use light blue in-content UI background
- Fix tab image.

Comment 9

[Brian Nicholson (:bnicholson)]

(In reply to Ian Barlow (:ibarlow) from comment #8)
> - Fix tab image.

That's happening because of bug 817828.

Comment 10

[Brian Nicholson (:bnicholson)]

Attachment: Part 1: Add about:privatebrowsing strings

There's still a bit of work to do with the about:privatebrowsing page to make it compatible with different device resolutions, and the assets aren't available yet. Can we just go with a strings-only patch and worry about the rest later? These are the strings shown in Ian's mockups.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 698314 [details] [diff] [review]
Part 1: Add about:privatebrowsing strings

Can we get a simple placeholder page ready to go soon? We want the l10n folks to be able to see the page in action to assist in the translation.

Comment 12

[Brian Nicholson (:bnicholson)]

Attachment: Part 1: Add about:privatebrowsing strings, v2

Ian's mockups show "You have opened a new private tab." in the same block as the private browsing description. I'm no l10n expert, but I think we'll need to make another entry that glues the sentences together, allowing RTL languages to reverse the order. I did this by adding the "privatebrowsingpage.description.private" entity.

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 698321 [details] [diff] [review]
Part 1: Add about:privatebrowsing strings, v2

The "private" mockup and the "not private" mockup treat the intro strings differently, which is a little odd.:

* "You have opened a new private tab." is part of the opening paragraph
* "You are not in a private tab." is a header, separate from the opening paragraph

The way you have the entities should work, but I think you could prepend the privatebrowsingpage.issueDesc.normal text directly to the privatebrowsingpage.description.private text and still make it work. Not a big deal I think.

Comment 14

[Ian Barlow (:ibarlow)]

Attachment: private browsing mask image

Hi Brian, here's the mask asset you can drop in - it's XHDPI, let me know if you need others.

Comment 15

[Brian Nicholson (:bnicholson)]

(In reply to Mark Finkle (:mfinkle) from comment #11)
> Comment on attachment 698314 [details] [diff] [review]
> Part 1: Add about:privatebrowsing strings
> 
> Can we get a simple placeholder page ready to go soon? We want the l10n
> folks to be able to see the page in action to assist in the translation.

Yeah, I'll try to get something up ASAP.

http://hg.mozilla.org/integration/mozilla-inbound/rev/fee24a580b1c

Comment 16

[Brian Nicholson (:bnicholson)]

(In reply to Ian Barlow (:ibarlow) from comment #14)
> Created attachment 698327 [details]
> private browsing mask image
> 
> Hi Brian, here's the mask asset you can drop in - it's XHDPI, let me know if
> you need others.

Thanks - do you also have the asset for the dark gray background image?

Comment 17

[Ian Barlow (:ibarlow)]

Attachment: about:privatebrowsing background image

Here you go Brian

Just note that this might not exactly match up in colour to the title bar background right now, but that it will shortly -- Sriram and I are still tweaking the title bar colour, so we just need to make a couple of changes to match it up to this background.

Comment 18

[Brian Nicholson (:bnicholson)]

Follow-up patch for missing angled bracket in DTD:

https://hg.mozilla.org/integration/mozilla-inbound/rev/c6d11daac3fb

Comment 19

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/fee24a580b1c
https://hg.mozilla.org/mozilla-central/rev/c6d11daac3fb

Comment 20

[Brian Nicholson (:bnicholson)]

Here's a build that should be pretty close to the mockups: http://dl.dropbox.com/u/35559547/fennec-about-pb.apk

Ian, can you take a look? Even if it's not perfect yet, it'd be good to land something now to help localizers (see comment 11).

Comment 21

[Ian Barlow (:ibarlow)]

This looks pretty awesome Brian. 

Weirdly, Roboto Light renders considerably lighter in weight on my phone than it does in Photoshop. We might want to try Roboto Regular in its place. I'm happy to move that into a different bug though if you need to land this.

Comment 22

[Brian Nicholson (:bnicholson)]

(In reply to Ian Barlow (:ibarlow) from comment #21)
> This looks pretty awesome Brian. 
> 
> Weirdly, Roboto Light renders considerably lighter in weight on my phone
> than it does in Photoshop. We might want to try Roboto Regular in its place.
> I'm happy to move that into a different bug though if you need to land this.

Yeah, I'm not sure what's happening there. On my Nexus, the font is rendered pixel-thin; on most of my other devices, though, it's actually quite a bit thicker than your mockup. Perhaps it's DPI related...

Comment 23

[Brian Nicholson (:bnicholson)]

Attachment: Part 2: Implement about:privatebrowsing

I was originally going to define a bunch of media queries to make this look correct on different devices, but I found that setting the "width=device-width, initial-scale=1;" has the effect of making px measurements into dip ones, simplifying the CSS. But without a fixed height for the content, there's no way that I know of to vertically center the page, so I'm still using media queries to set top margins for various resolutions.

Comment 24

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 698830 [details] [diff] [review]
Part 2: Implement about:privatebrowsing

>diff --git a/mobile/android/base/BrowserToolbar.java b/mobile/android/base/BrowserToolbar.java

>-        if (tab != null && "about:home".equals(tab.getURL()))
>+        if (tab != null && ("about:home".equals(tab.getURL()) ||
>+                            "about:privatebrowsing".equals(tab.getURL())))

I smell a whitelist in the future. Any more "about:xxx" and this will be too unwieldy
 
>diff --git a/mobile/android/components/AboutRedirector.js b/mobile/android/components/AboutRedirector.js

>+  privatebrowsing: {
>+    uri: "chrome://browser/content/aboutPrivateBrowsing.xhtml",
>+    privileged: true

Might be nice to try to get this unprivileged someday

>diff --git a/mobile/android/themes/core/images/privatebrowsing-mask.png b/mobile/android/themes/core/images/privatebrowsing-mask.png

This is a big file, I think. How big on disk? I wonder if Ian "crunched" it.

r+, looks good for now

Comment 25

[Brian Nicholson (:bnicholson)]

(In reply to Mark Finkle (:mfinkle) from comment #24)
> >diff --git a/mobile/android/themes/core/images/privatebrowsing-mask.png b/mobile/android/themes/core/images/privatebrowsing-mask.png
> 
> This is a big file, I think. How big on disk?
> 

Looks like it's 35.6kB.

Comment 26

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/960e37ed0a59

Comment 27

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/960e37ed0a59

Comment 28

[Brian Nicholson (:bnicholson)]

Comment on attachment 698830 [details] [diff] [review]
Part 2: Implement about:privatebrowsing

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new feature
User impact if declined: creating a new private tab will just show about:home
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low risk (mostly just adds a new html page)
String or UUID changes made by this patch: none (strings were landed separately in part 1)

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ad13c5d937f3

Comment 30

[Andreea Pod]

Verified fixed on:
-build: Firefox for Android 20.0a2 (2013-01-14), Firefox for Android 21.0a1 (2013-01-14)
-device: Samsung Galaxy Nexus 
-OS: Android 4.1.2

Comment 31

[Andreea Pod]

Test case added: https://moztrap.mozilla.org/manage/case/6090/