1175970 - Tell users about tracking protection the first time they open a new private tab

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

antlam has a mock-up for this.

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_mob_frue2.png

Same, UI and specs as the one we used for Tab queue!

Comment 2

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: mov_frue2.mov

Leaving a brief clip of the interaction here too, just in case.

Comment 3

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Note: we can reuse the same Tracking protection shield icon from bug 1147112.

20dp above the title, 40dp down from the top of the sheet.

Comment 4

[Martyn Haigh (:mhaigh)]

I've got a preview here https://dl.dropboxusercontent.com/u/7163922/1175970.apk.  Does that look ok to you, antlam?

Comment 5

[Martyn Haigh (:mhaigh)]

Attachment: MozReview Request: Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?margaret

Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?sebastian

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8630526 [details]
MozReview Request: Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?margaret

https://reviewboard.mozilla.org/r/12759/#review11385

::: mobile/android/base/moz.build:491
(Diff revision 1)
> +    'trackingprotection/TrackingProtectionPrompt.java',

This file seems to be missing in this commit. The build fails locally for me: make[5]: *** No rule to make target `trackingprotection/TrackingProtectionPrompt.java', needed by `gecko-browser.jar'.  Stop.

::: mobile/android/base/BrowserApp.java:2071
(Diff revision 1)
> +                boolean hasTrackingProtectionPromptBeShownBefore = prefs.getBoolean(GeckoPreferences.PREFS_TRACKING_PROTECTION_PROMPT_SHOWN, false);

I wonder if we could unify hasTrackingProtectionPromptBeShownBefore and mTrackingProtectionPromptHasBeenShown. Usually both should be true or both are false, right? Could we use the same boolean?

As SharedPreferences instances are static singletons (inside Context) after loading, we actually could avoid having booleans altogether and just read the preference everytime (It's just a hashmap lookup and we need to load it once anyways.). What do you think?

Comment 7

[Martyn Haigh (:mhaigh)]

Comment on attachment 8630526 [details]
MozReview Request: Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?margaret

Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?sebastian

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8630526 [details]
MozReview Request: Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?margaret

https://reviewboard.mozilla.org/r/12759/#review11407

Ship It!

Comment 9

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=070aec6c3a4f

Comment 10

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=86aedab704ad

Comment 11

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=933349e88518

Comment 12

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=60289fc81436

Comment 13

[Martyn Haigh (:mhaigh)]

Comment on attachment 8630526 [details]
MozReview Request: Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?margaret

Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?margaret

Sebastian has reviewed the main code, but it was failing some tests, so I've updated.  I'm not 100% sure about the changes to the tests as it assumes that every private tab will encounter the slide in notification.  This assumption seems to hold true from my try runs.

Comment 14

[:Margaret Leibovic]

(In reply to Martyn Haigh (:mhaigh) from comment #13)
> Comment on attachment 8630526 [details]
> MozReview Request: Bug 1175970 - Tell users about tracking protection the
> first time they open a new private tab; r?margaret
> 
> Bug 1175970 - Tell users about tracking protection the first time they open
> a new private tab; r?margaret
> 
> Sebastian has reviewed the main code, but it was failing some tests, so I've
> updated.  I'm not 100% sure about the changes to the tests as it assumes
> that every private tab will encounter the slide in notification.  This
> assumption seems to hold true from my try runs.

I wonder if that's because we only ever open at most one private tab per test. Every robocop test runs with a clean profile, so any "we already showed this" pref would get reset.

I know that we can change gecko preferences in the testing profile, but I wonder if we can also set shared preferences, such that we can turn off this notification in all tests.

I'm surprised we only need to fix this in PixelTest... but I guess looking at our test code, we only seem to test private tabs in these two tests:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/tests/browser/robocop/testPrivateBrowsing.java
http://mxr.mozilla.org/mozilla-central/source/mobile/android/tests/browser/robocop/testFilterOpenTab.java

The first one extends PixelTest, and the second one doesn't do any UI testing.

I'm fine with us landing this fix in PixelTest for now, but it seems really fragile if we ever add a new PixelTest that opens multiple private tabs, or if we add another non-PixelTest that opens a private tab.

I think you should file a follow-up bug to set the PREFS_TRACKING_PROTECTION_PROMPT_SHOWN pref to true. And you could also file a bug to add a test that sets that to false, opens a new private tab, and makes sure the prompt is shown. Then that test could close that tab and open a new private tab to make sure the prompt *wasn't* shown :)

Comment 15

[:Margaret Leibovic]

Comment on attachment 8630526 [details]
MozReview Request: Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?margaret

https://reviewboard.mozilla.org/r/12759/#review11921

r+ for the testing changes, provided you file (and fix) a follow-up bug to make this better.

Comment 16

[Martyn Haigh (:mhaigh)]

url:        https://hg.mozilla.org/integration/fx-team/rev/eb346fb7c9627320015215fd40b024d3b1fac1fb
changeset:  eb346fb7c9627320015215fd40b024d3b1fac1fb
user:       Martyn Haigh <[e-mail]>
date:       Wed Jul 15 11:23:37 2015 +0100
description:
Bug 1175970 - Tell users about tracking protection the first time they open a new private tab r=margaret

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/eb346fb7c962

Comment 18

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot_2015-07-22-10-37-33.png

Tested with:
Device: Nexus 4 (Android 5.1)
Build: Firefox for Android 42.0a1 (2015-07-20)

Comment 19

[Teodora Vermesan (:TeoVermesan)]

If you are in normal browsing, long tap on a website from top sites, choose to open it in a new private tab, switch to it. And after that, open tab tray and tap the "+" button to open a new private tab.
Now you have 2 private tabs and the info about tracking protection appears when you open the second one. Is it expected? http://i.imgur.com/UCiB65K.png

Comment 20

[Martyn Haigh (:mhaigh)]

This is not expected, but we're actually planning on changing this feature to merge it with the contextual hint.  See bug 1177612 comment 29 for more info.