1524862 - New content blocking UI doesn't reflect EnableTrackingProtection policy

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, defect, P1)

Description

[Sören Hentzschel]

STR:

1. enable the EnableTrackingProtection policy
2. open the privacy settings

Expected:

As in Firefox 64 the privacy settings should show that the tracking protection is always enabled. Firefox 65 has a new UI. It's expected that the "custom" settings are enabled where the "trackers" dropdown shows "in all windows".

Actual:

The "trackers" dropdown in the "custom" section shows "in all windows" in the new privacy settings but the "default" settings are selected, telling that tracking protection is enabled only in private windows which is wrong.

I didn't test if the policy works as expected and if it's only a visual bug. But even if it's only a visual bug it should be fixed of course. :)

Comment 1

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

Johann, is this something that you can look in to?

Comment 2

[Johann Hofmann [:johannh]]

Yup!

Comment 3

[Tanvi Vyas[:tanvi]]

Johann, does this need to be uplifted to 66?

Comment 4

[Tim Spurway [:tspurway]]

as per comment 3. johann, should we expect a patch for 66?

Comment 5

[Johann Hofmann [:johannh]]

Please don't just assign people to bugs. Erica, can you take a look at this whenever you have time? Thanks!

Comment 6

[Erica Wright [:ewright]]

Hi Soren, I tested this and it seems that it is strictly a visual bug. "standard" will detect the default value of the prefs, and since the EnableTrackingProtection policy changes the default the user is placed there. You are correct that when you click on "custom" you can see the correct policy, changing to "custom" does not change any of the prefs so you can use that to see what is actually applying.
We should however change the copy, or put the user in custom automatically for these cases. I'll start a conversation with UX about this.

Comment 7

[Johann Hofmann [:johannh]]

This also seems like something that could easily be tested.

Comment 8

[Tim Spurway [:tspurway]]

How's this going, Erica? Do you think we should expect anything for 66?

Comment 9

[Erica Wright [:ewright]]

(In reply to Tim Spurway [:tspurway] from comment #8)

How's this going, Erica? Do you think we should expect anything for 66?

I talked with UX and the consensus was that the category should be matched with our pre-defined categories if it perfectly matches, otherwise, it should move into custom. We haven't talked about what disabled categories will look like.

I'll start working on this shortly. I'm not sure how long it will take yet.

Comment 10

[Erica Wright [:ewright]]

Attachment: Bug 1524862 - Enforce expected category prefs even if enterprise policy changes the defaults.

Do not rely on default values when matching categories, enforce expected prefs.

Comment 11

[Erica Wright [:ewright]]

Attachment: Bug 1524862 - Enforce expected category prefs even if enterprise policy changes the defaults.

Enforce expected category prefs even if enterprise policy changes the defaults. Lock user in custom if one of the relevant policy prefs are locked.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad25e831bd26
Enforce expected category prefs even if enterprise policy changes the defaults. r=johannh

Comment 13

[Razvan Maries]

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

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Is this something we should uplift to 66 (noting that RC week is next week) or can it ride to release with 67?

Comment 15

[Erica Wright [:ewright]]

Comment on attachment 9046489 [details]
Bug 1524862 - Enforce expected category prefs even if enterprise policy changes the defaults.

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1501985
• User impact if declined: Users who have a custom policy set involving cookies or tracking protection will have misleading UI that may lead them to believe that their policy is not being correctly set.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): changes a superficial pref which affects the way the privacy preferences is displayed to the user. prevents misleading UI. These are strictly visual changes which better reflect how the setting are actually behaving.
• String changes made/needed: none

Comment 16

[Erica Wright [:ewright]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #14)

Is this something we should uplift to 66 (noting that RC week is next week) or can it ride to release with 67?

sure, so long as it isn't too late.

Comment 17

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

I do feel it's a bit late for now. I might take it if we had some QA verification or if this were a bit earlier than the very last minute in the cycle. It looks like there are some string changes as well so this is probably best left to ride with 67.

Comment 18

[Erica Wright [:ewright]]

(In reply to Liz Henry (:lizzard) (use needinfo) from comment #17)

I do feel it's a bit late for now. I might take it if we had some QA verification or if this were a bit earlier than the very last minute in the cycle. It looks like there are some string changes as well so this is probably best left to ride with 67.

Not a problem, thank you.