1527151 - Content Blocking shield and subpanels persist after switching tabs

Status: VERIFIED FIXED

(Firefox :: Site Identity, defect, P1)

Description

[Steven Englehardt [:englehardt]]

STR:

1. Go to https://senglehardt.com/test/trackingprotection/test_pages/index.html. Note the lack of shield.
2. Click "Fingerprinting and Cryptomining and Trackers" in the same tab.
3. Click the back button
4. Right click "Fingerprinting and Cryptomining and Trackers" and open in a new tab.
5. Switch back to the original tab

Expected:
Original tab does not show the shield

Actual:
Original tab show the shield state of second tab. The rows for cryptomining/fingerprinting are still present, but the subpanels are (correctly) empty.

Comment 1

[(no longer active)]

Content blocking log at step 5:

await gBrowser.selectedBrowser.getContentBlockingLog()
"{
"https://senglehardt.com\": [[32768, true, 1]]
}
"

The UI seems to be highly confused here. Johann, any chance you can take a look please?

Comment 2

[Johann Hofmann [:johannh]]

Quite weird, I can reproduce and will take a look.

Comment 3

[Johann Hofmann [:johannh]]

Attachment: Bug 1527151 - Reset securityUI.contentBlockingEvent on top level location changes. r=Ehsan

Comment 4

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=47b737d236ed10710ce02c5b55a27e286f177a05

Comment 5

[Johann Hofmann [:johannh]]

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d945b5bd54f0
Reset securityUI.contentBlockingEvent on top level location changes. r=Ehsan

Comment 7

[(no longer active)]

This patch touches code which Barret may be working on now...

Comment 8

[Razvan Maries]

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

Comment 9

[Razvan Maries]

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

Comment 10

[Johann Hofmann [:johannh]]

Comment on attachment 9044910 [details]
Bug 1527151 - Reset securityUI.contentBlockingEvent on top level location changes. r=Ehsan

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1520879
• User impact if declined: Benign pages may be displayed as containing trackers
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a relatively compact and straightforward fix that has a test and has been baking in Nightly for a week now
• String changes made/needed: None

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9044910 [details]
Bug 1527151 - Reset securityUI.contentBlockingEvent on top level location changes. r=Ehsan

Fix for new regression in 66, has been on nightly a while, includes tests.
Let's uplift for beta 13.

Comment 12

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bdc420ea17d6

Comment 13

[Maria Berlinger [:mberlinger], Fxa/Subplat]

I've managed to reproduce this bug on an affected Firefox 66.0b7 using Windows 10x64.

This issue is verified fixed on Firefox 66.0b13 and Firefox 67.0a1 (20190303215808) using Windos 10x64 and macOS 10.13 but in Step 4 from comment 0 the shield doesn't appear just after refreshing the page, is this intended?

Comment 14

[Johann Hofmann [:johannh]]

(In reply to Maria Berlinger [:maria_berlinger], Release Desktop QA from comment #13)

I've managed to reproduce this bug on an affected Firefox 66.0b7 using Windows 10x64.

This issue is verified fixed on Firefox 66.0b13 and Firefox 67.0a1 (20190303215808) using Windos 10x64 and macOS 10.13 but in Step 4 from comment 0 the shield doesn't appear just after refreshing the page, is this intended?

Yeah, that seems like a bug, can you please file it? Thanks!

Comment 15

[Maria Berlinger [:mberlinger], Fxa/Subplat]

Logged bug 1533362.
Closing this as verified fixed per comment 14