Closed Bug 1175970 Opened 5 years ago Closed 4 years ago

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

Categories

(Firefox for Android :: General, defect)

35 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 42
Tracking Status
firefox42 --- verified

People

(Reporter: Margaret, Assigned: mhaigh)

References

Details

Attachments

(4 files)

antlam has a mock-up for this.
Flags: needinfo?(alam)
Attached image prev_mob_frue2.png
Same, UI and specs as the one we used for Tab queue!
Flags: needinfo?(alam)
Attached video mov_frue2.mov
Leaving a brief clip of the interaction here too, just in case.
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.
Assignee: nobody → mhaigh
Status: NEW → ASSIGNED
I've got a preview here https://dl.dropboxusercontent.com/u/7163922/1175970.apk.  Does that look ok to you, antlam?
Depends on: 1174242
Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?sebastian
Attachment #8630526 - Flags: review?(s.kaspari)
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?
Attachment #8630526 - Flags: review?(s.kaspari)
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
Attachment #8630526 - Flags: review?(s.kaspari)
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!
Attachment #8630526 - Flags: review?(s.kaspari) → review+
Attachment #8630526 - Attachment description: MozReview Request: Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?sebastian → MozReview Request: Bug 1175970 - Tell users about tracking protection the first time they open a new private tab; r?margaret
Attachment #8630526 - Flags: review+ → review?(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

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.
(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 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.
Attachment #8630526 - Flags: review?(margaret.leibovic) → review+
Blocks: 1184046
Blocks: 1184048
url:        https://hg.mozilla.org/integration/fx-team/rev/eb346fb7c9627320015215fd40b024d3b1fac1fb
changeset:  eb346fb7c9627320015215fd40b024d3b1fac1fb
user:       Martyn Haigh <mhaigh@mozilla.org>
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
https://hg.mozilla.org/mozilla-central/rev/eb346fb7c962
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Tested with:
Device: Nexus 4 (Android 5.1)
Build: Firefox for Android 42.0a1 (2015-07-20)
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
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.
You need to log in before you can comment on or make changes to this bug.