Closed Bug 1487396 Opened 6 years ago Closed 6 years ago

Update the Control Centre to not show the shield icon unless something is really blocked

Categories

(Firefox :: Site Identity, defect, P1)

Product:
Firefox
Component:
Site Identity
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 64
Tracking Flags:
Tracking Status
firefox62 --- unaffected
firefox63 --- verified
firefox64 --- verified
firefox65 --- verified

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

(Blocks 1 open bug)

Details

(Whiteboard: [privacy-panel-64])

Attachments

(6 files)

Bug 1487396 - Part 1: Enable the secure browser UI object to keep track of the newly added content blocking states for each document; r=baku
46 bytes, text/x-phabricator-request
baku
: review+
pascalc
: approval-mozilla-beta+
Details | Review
Bug 1487396 - Part 2: Only show the shield icon when something gets blocked through Content Blocking; r=johannh
46 bytes, text/x-phabricator-request
johannh
: review+
Details | Review
Bug 1487396 - Part 3: Test that before enabling tracking protection, the shield doesn't show up unless tracking elements are blocked; r=johannh
46 bytes, text/x-phabricator-request
johannh
: review+
Details | Review
Bug 1487396 - Part 4: Test that when fastblock is enabled, the shield only shows up when tracking elements are blocked; r=johannh
46 bytes, text/x-phabricator-request
johannh
: review+
Details | Review
Bug 1487396 - Part 5: Test that when third-party cookie blocking is enabled, the shield only shows up when tracking elements are blocked; r=johannh
46 bytes, text/x-phabricator-request
johannh
: review+
Details | Review
Bug 1487396 - Part 6: Turn the FastBlock pref off during the test runs where we were previously relying on it being off by default; r=johannh
46 bytes, text/x-phabricator-request
johannh
: review+
Details | Review 
      No description provided.
Check STATE_BLOCKED_TRACKING_COOKIES and STATE_COOKIES_BLOCKED_FOREIGN.  Plus the FastBlock and TP notifications.
(In reply to Tanvi Vyas[:tanvi] from comment #1)
> Check STATE_BLOCKED_TRACKING_COOKIES and STATE_COOKIES_BLOCKED_FOREIGN. 
> Plus the FastBlock and TP notifications.

Yep, that is the plan...
Priority: -- → P2
We want this in 64 and very likely in 63.
status-firefox62: --- → unaffected
status-firefox63: --- → affected
Whiteboard: [privacy-panel-64]
Ehsan, it seems like you're already assigned. Let me know if you want me to take this one, otherwise I'm happy to review.
Status: NEW → ASSIGNED
Depends on: 1487390
Priority: P2 → P1
Depends on D4809
I will submit a patch with more tests later, perhaps in bug 1485318 or here in some form...
Thanks for writing these tests, I assume this replaces bug 1485318?
Yes, I think so, there is really no more useful tests to add there which I can think of...
review ping?  This needs to land ASAP.
Flags: needinfo?(jhofmann)
Flags: needinfo?(amarchesini)
Comment on attachment 9005777 [details]
Bug 1487396 - Part 1: Enable the secure browser UI object to keep track of the newly added content blocking states for each document; r=baku

Andrea Marchesini [:baku] has approved the revision.
Attachment #9005777 - Flags: review+
Comment on attachment 9005778 [details]
Bug 1487396 - Part 2: Only show the shield icon when something gets blocked through Content Blocking; r=johannh

Johann Hofmann [:johannh] has approved the revision.
Attachment #9005778 - Flags: review+
Comment on attachment 9005785 [details]
Bug 1487396 - Part 3: Test that before enabling tracking protection, the shield doesn't show up unless tracking elements are blocked; r=johannh

Johann Hofmann [:johannh] has approved the revision.
Attachment #9005785 - Flags: review+
Comment on attachment 9005786 [details]
Bug 1487396 - Part 4: Test that when fastblock is enabled, the shield only shows up when tracking elements are blocked; r=johannh

Johann Hofmann [:johannh] has approved the revision.
Attachment #9005786 - Flags: review+
Comment on attachment 9005787 [details]
Bug 1487396 - Part 5: Test that when third-party cookie blocking is enabled, the shield only shows up when tracking elements are blocked; r=johannh

Johann Hofmann [:johannh] has approved the revision.
Attachment #9005787 - Flags: review+
Comment on attachment 9006290 [details]
Bug 1487396 - Part 6: Turn the FastBlock pref off during the test runs where we were previously relying on it being off by default; r=johannh

Johann Hofmann [:johannh] has approved the revision.
Attachment #9006290 - Flags: review+
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc2beb31a3d4
Part 1: Enable the secure browser UI object to keep track of the newly added content blocking states for each document; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/9dc6b07c3d65
Part 2: Only show the shield icon when something gets blocked through Content Blocking; r=johannh
https://hg.mozilla.org/integration/mozilla-inbound/rev/1936a83c2e78
Part 3: Test that before enabling tracking protection, the shield doesn't show up unless tracking elements are blocked; r=johannh
https://hg.mozilla.org/integration/mozilla-inbound/rev/1928d891a3f3
Part 4: Test that when fastblock is enabled, the shield only shows up when tracking elements are blocked; r=johannh
https://hg.mozilla.org/integration/mozilla-inbound/rev/902a3eb6c32b
Part 5: Test that when third-party cookie blocking is enabled, the shield only shows up when tracking elements are blocked; r=johannh
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a4199857e74
Part 6: Turn the FastBlock pref off during the test runs where we were previously relying on it being off by default; r=johannh
Flags: needinfo?(jhofmann)
Flags: needinfo?(amarchesini)
Comment on attachment 9005777 [details]
Bug 1487396 - Part 1: Enable the secure browser UI object to keep track of the newly added content blocking states for each document; r=baku

Approval Request Comment
[Feature/Bug causing the regression]: This is not a regression, we are changing the behavior of the shield icon in the location bar.
[User impact if declined]: Things like bug 1485827 when one of the content blocking options is enabled.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Just landed in inbound.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: The first part of the patch makes changes that impact some of the behavior of the browser with its default configuration on the beta channel, but the changes are mostly keeping track of state, so should be relatively low risk.  The other changes in part 1, as well as part 2 (the rest are just tests) depend on the Content Blocking UI pref which is disabled on late beta for now, so the changes will ultimately not affect the released product.  That being said, because this is a major change to the behavior of the shield icon, we would like to uplift it to beta sooner than later in order to improve the user experience when using content blocking.
[Why is the change risky/not risky?]: See above.
[String changes made/needed]: None.
Attachment #9005777 - Flags: approval-mozilla-beta?
(BTW the approval request is for all the attached patches.)
Comment on attachment 9005777 [details]
Bug 1487396 - Part 1: Enable the secure browser UI object to keep track of the newly added content blocking states for each document; r=baku

Approved for 63 beta 4
Attachment #9005777 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Checkins from comment 27 had been backed out in https://bugzilla.mozilla.org/show_bug.cgi?id=1487396 Relanded in comment 28.
Will this make it to beta 4?
Depends on: 1489525
Verified this issue On Nightly 65.0a1 (2018-11-02), Firefox 63.0.1 and Firefox 64.0b6, on Windows 10, Ubuntu 16.04 and Mac.
Status: RESOLVED → VERIFIED
status-firefox63: fixed → verified
status-firefox64: fixed → verified
status-firefox65: --- → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: