TP animation is shown every time a new video segment is loaded

VERIFIED FIXED in Firefox 64

Status

()

defect
P1
normal
VERIFIED FIXED
11 months ago
5 months ago

People

(Reporter: darkspirit, Assigned: Ehsan)

Tracking

({nightly-community, polish, regression})

Trunk
mozilla65
x86_64
All
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(geckoview62 unaffected, firefox-esr60 unaffected, firefox62 unaffected, firefox63 unaffected, firefox64+ verified, firefox65 verified)

Details

Attachments

(3 attachments)

STR:
1. have Always-on Tracking Protection + block 3rd-party cookies
2. Watch or just seek through the video: https://www.zdf.de/comedy/heute-show/heute-show-vom-5-oktober-2018-100.html

Actual: The TP animation is shown every time a new video segment is loaded.

Expected: The TP animation is shown only once per real page load.

mozregression --good 2018-09-15 --bad 2018-10-06 --pref privacy.trackingprotection.enabled:true network.cookie.cookieBehavior:1 -a https://www.zdf.de/comedy/heute-show/heute-show-vom-5-oktober-2018-100.html
> 14:08.85 INFO: Last good revision: 1681bd622c63ad7cf045cb70d487e0b538564d3f
> 14:08.85 INFO: First bad revision: 8db78a0b0d4f7b2921f4779306d933168effef1a
> 14:08.85 INFO: Pushlog:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1681bd622c63ad7cf045cb70d487e0b538564d3f&tochange=8db78a0b0d4f7b2921f4779306d933168effef1a

> 8db78a0b0d4f	Ehsan Akhgari — Bug 1493563
Flags: needinfo?(ehsan)
Ah, this is a dupe of bug 1495207, but thank you for finding the regressor!
Status: NEW → RESOLVED
Closed: 10 months ago
Flags: needinfo?(ehsan)
Resolution: --- → DUPLICATE
Duplicate of bug: 1495207
No longer blocks: 1493563
Status: RESOLVED → REOPENED
OS: Linux → All
Resolution: DUPLICATE → ---
Whiteboard: [ux-decision-needed]
If you wanted to fix this problem as a whole and not only for videos, please feel free to re-dupe to bug 1495207. It was wontfixed because it wasn't a regression from an implementation viewpoint, but it's the original report of this general UX regression.
[Tracking Requested - why for this release]:

Wait, if this is regressed by bug 1493563, how can it possibly be a duplicate of bug 1495207, given that that bug was *not* regressed by bug 1493563?!

Jan, I assume you are certain about the regression range in comment 0, right?  De-duping for now and marking this as a real regression to be tracked and fixed.
Blocks: 1493563
Whiteboard: [ux-decision-needed]
Assignee: nobody → ehsan
(In reply to :Ehsan Akhgari from comment #3)
> Jan, I assume you are certain about the regression range in comment 0, right?  

Yes, re-confirmed:

last good:
mozregression --repo mozilla-inbound --launch 1681bd622c63ad7cf045cb70d487e0b538564d3f --pref privacy.trackingprotection.enabled:true network.cookie.cookieBehavior:1 -a https://www.zdf.de/comedy/heute-show/heute-show-vom-5-oktober-2018-100.html

first bad:
mozregression --repo mozilla-inbound --launch 8db78a0b0d4f7b2921f4779306d933168effef1a --pref privacy.trackingprotection.enabled:true network.cookie.cookieBehavior:1 -a https://www.zdf.de/comedy/heute-show/heute-show-vom-5-oktober-2018-100.html
Thanks!  I'll try to have a look soon.
Keywords: polish
Priority: -- → P1
The thing that this test case has special about it is that we alternate between receiving STATE_BLOCKED_TRACKING_CONTENT and STATE_COOKIES_BLOCKED_TRACKER notifications (due to TP and cookie blocking).  Every time we transition from one notification type to the other, the shield animates.  When we receive repeated notifications of the same type, no animation occurs.

Here is an excerpt from the content blocking log reported in the web console where this bimodal behavior can be seen:

Request to access cookie or storage on “https://zdfvodnone-vh.akamaihd.net/i/meta-files/zdf/smil/m3u8/300/18/10/181005_sendung_hsh/3/181005_sendung_hsh.smil/segment51_3…” was blocked because we are blocking all third-party storage access requests and content blocking is enabled. heute-show-vom-5-oktober-2018-100.html
Request to access cookie or storage on “https://zdfvodnone-vh.akamaihd.net/i/meta-files/zdf/smil/m3u8/300/18/10/181005_sendung_hsh/3/181005_sendung_hsh.smil/segment52_3…” was blocked because we are blocking all third-party storage access requests and content blocking is enabled. heute-show-vom-5-oktober-2018-100.html
The resource at “https://logs1407.xiti.com/hit.xiti?s=569006&ts=1540594324918&vtag=5.13.2&ptag=js&r=1920x1080x24x24&re=1280x872&hl=18x52x4&lng=en-US&plyr=0&p=heute-show_vom_5._Oktober_2018&a=refresh&type=video&s2=Comedy&m1=2207&m5=int&m6=clip” was blocked because content blocking is enabled.[Learn More] heute-show-vom-5-oktober-2018-100.html
Request to access cookie or storage on “https://zdfvodnone-vh.akamaihd.net/i/meta-files/zdf/smil/m3u8/300/18/10/181005_sendung_hsh/3/181005_sendung_hsh.smil/segment53_3…” was blocked because we are blocking all third-party storage access requests and content blocking is enabled. heute-show-vom-5-oktober-2018-100.html
Request to access cookie or storage on “https://zdfvodnone-vh.akamaihd.net/i/meta-files/zdf/smil/m3u8/300/18/10/181005_sendung_hsh/3/181005_sendung_hsh.smil/segment54_3…” was blocked because we are blocking all third-party storage access requests and content blocking is enabled. heute-show-vom-5-oktober-2018-100.html
Request to access cookie or storage on “https://zdfvodnone-vh.akamaihd.net/i/meta-files/zdf/smil/m3u8/300/18/10/181005_sendung_hsh/3/181005_sendung_hsh.smil/segment55_3…” was blocked because we are blocking all third-party storage access requests and content blocking is enabled. heute-show-vom-5-oktober-2018-100.html
The resource at “https://logs1407.xiti.com/hit.xiti?s=569006&ts=1540594354920&vtag=5.13.2&ptag=js&r=1920x1080x24x24&re=1280x872&hl=18x52x34&lng=en-US&plyr=0&p=heute-show_vom_5._Oktober_2018&a=refresh&type=video&s2=Comedy&m1=2207&m5=int&m6=clip” was blocked because content blocking is enabled.[Learn More] heute-show-vom-5-oktober-2018-100.html
Request to access cookie or storage on “https://zdfvodnone-vh.akamaihd.net/i/meta-files/zdf/smil/m3u8/300/18/10/181005_sendung_hsh/3/181005_sendung_hsh.smil/segment56_3…” was blocked because we are blocking all third-party storage access requests and content blocking is enabled. heute-show-vom-5-oktober-2018-100.html
Request to access cookie or storage on “https://zdfvodnone-vh.akamaihd.net/i/meta-files/zdf/smil/m3u8/300/18/10/181005_sendung_hsh/3/181005_sendung_hsh.smil/segment57_3…” was blocked because we are blocking all third-party storage access requests and content blocking is enabled. heute-show-vom-5-oktober-2018-100.html
Request to access cookie or storage on “https://zdfvodnone-vh.akamaihd.net/i/meta-files/zdf/smil/m3u8/300/18/10/181005_sendung_hsh/3/181005_sendung_hsh.smil/segment58_3…” was blocked because we are blocking all third-party storage access requests and content blocking is enabled. heute-show-vom-5-oktober-2018-100.html
The resource at “https://logs1407.xiti.com/hit.xiti?s=569006&ts=1540594384923&vtag=5.13.2&ptag=js&r=1920x1080x24x24&re=1280x872&hl=18x53x4&lng=en-US&plyr=0&p=heute-show_vom_5._Oktober_2018&a=refresh&type=video&s2=Comedy&m1=2207&m5=int&m6=clip” was blocked because content blocking is enabled.[Learn More] heute-show-vom-5-oktober-2018-100.html
Request to access cookie or storage on “https://zdfvodnone-vh.akamaihd.net/i/meta-files/zdf/smil/m3u8/300/18/10/181005_sendung_hsh/3/181005_sendung_hsh.smil/segment59_3…” was blocked because we are blocking all third-party storage access requests and content blocking is enabled. heute-show-vom-5-oktober-2018-100.html
Request to access cookie or storage on “https://zdfvodnone-vh.akamaihd.net/i/meta-files/zdf/smil/m3u8/300/18/10/181005_sendung_hsh/3/181005_sendung_hsh.smil/segment60_3…” was blocked because we are blocking all third-party storage access requests and content blocking is enabled. heute-show-vom-5-oktober-2018-100.html
Request to access cookie or storage on “https://zdfvodnone-vh.akamaihd.net/i/meta-files/zdf/smil/m3u8/300/18/10/181005_sendung_hsh/3/181005_sendung_hsh.smil/segment61_3…” was blocked because we are blocking all third-party storage access requests and content blocking is enabled. heute-show-vom-5-oktober-2018-100.html
The resource at “https://logs1407.xiti.com/hit.xiti?s=569006&ts=1540594414924&vtag=5.13.2&ptag=js&r=1920x1080x24x24&re=1280x872&hl=18x53x34&lng=en-US&plyr=0&p=heute-show_vom_5._Oktober_2018&a=refresh&type=video&s2=Comedy&m1=2207&m5=int&m6=clip” was blocked because content blocking is enabled.[Learn More]
That was STATE_COOKIES_BLOCKED_FOREIGN in comment 6, not STATE_COOKIES_BLOCKED_TRACKER...
I found the cause of the bug, it basically stems from the fact that we can no longer remember that a document has received STATE_BLOCKED_TRACKING_CONTENT.

Here is why.  Once we receive STATE_BLOCKED_TRACKING_CONTENT, we get to this code: <https://searchfox.org/mozilla-central/rev/d5fc6f3d4746279a7c6fd4f7a98b1bc5d05d6b01/dom/base/nsGlobalWindowOuter.cpp#5329>.  This is now implemented in terms of ContentBlockingLog <https://searchfox.org/mozilla-central/rev/d5fc6f3d4746279a7c6fd4f7a98b1bc5d05d6b01/dom/base/nsIDocument.h#1053> (which calls into <https://searchfox.org/mozilla-central/rev/d5fc6f3d4746279a7c6fd4f7a98b1bc5d05d6b01/dom/base/ContentBlockingLog.h#52>.  But the problem here is that our origin is empty!  So RecordLog() doesn't actually record anything in the content blocking log, and just bails out.

Then we get to here later on when we're about to dispatch OnSecurityChange <https://searchfox.org/mozilla-central/rev/d5fc6f3d4746279a7c6fd4f7a98b1bc5d05d6b01/security/manager/ssl/nsSecureBrowserUIImpl.cpp#181>.  This brings us to here <https://searchfox.org/mozilla-central/rev/d5fc6f3d4746279a7c6fd4f7a98b1bc5d05d6b01/dom/base/nsIDocument.h#998> which in turn brings us to <https://searchfox.org/mozilla-central/rev/d5fc6f3d4746279a7c6fd4f7a98b1bc5d05d6b01/dom/base/ContentBlockingLog.h#116>.  But this method isn't going to work if our content blocking log is empty for this notification type, which it will be because of the above early return.
Component: Tracking Protection → DOM
Product: Firefox → Core
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a84a792f57d3
Part 1: Pass in a URI hint for all content blocking state notifications r=baku
https://hg.mozilla.org/integration/autoland/rev/3e52e8b6befb
Part 2: Extend browser_trackingUI_animation_2.js to cover third-party cookie blocking too r=johannh
https://hg.mozilla.org/integration/autoland/rev/b90fa929d204
Part 3: Add a test to ensure that the shield doesn't reanimate when seeing notifications for existing blocking categories r=johannh
https://hg.mozilla.org/mozilla-central/rev/a84a792f57d3
https://hg.mozilla.org/mozilla-central/rev/3e52e8b6befb
https://hg.mozilla.org/mozilla-central/rev/b90fa929d204
Status: REOPENED → RESOLVED
Closed: 10 months ago10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Flags: qe-verify+
Flags: in-testsuite+
Verified fixed on Debian Testing. Ready to uplift. Thank you! :)

mozregression --find-fix --bad 2018-10-28 --good 2018-10-30 --pref privacy.trackingprotection.enabled:true network.cookie.cookieBehavior:1 -a https://www.zdf.de/comedy/heute-show/heute-show-vom-5-oktober-2018-100.html
> 12:57.25 INFO: First good revision: b90fa929d204233cb4e79b96363dcfac1571faf1
> 12:57.25 INFO: Last bad revision: a1f9c46668550d18322337e43b361e48a28694dd
> 12:57.25 INFO: Pushlog:
> https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a1f9c46668550d18322337e43b361e48a28694dd&tochange=b90fa929d204233cb4e79b96363dcfac1571faf1

> b90fa929d204	Ehsan Akhgari — Bug 1497014 - Part 3: Add a test to ensure that the shield doesn't reanimate when seeing notifications for existing blocking categories r=johannh
> 3e52e8b6befb	Ehsan Akhgari — Bug 1497014 - Part 2: Extend browser_trackingUI_animation_2.js to cover third-party cookie blocking too r=johannh
> a84a792f57d3	Ehsan Akhgari — Bug 1497014 - Part 1: Pass in a URI hint for all content blocking state notifications r=baku
Status: RESOLVED → VERIFIED
Thank you for the great bug report, and for helping to verify the fix!  :-)
Comment on attachment 9020507 [details]
Bug 1497014 - Part 1: Pass in a URI hint for all content blocking state notifications

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1493563

User impact if declined: See comment 0.  This problem could happen on many other websites too.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

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): This is quite low-risk.  The fix in bug 1493563 was almost complete except for this last edge case which this patch is fixing.  And it's effectively passing in a boolean and an nsIURI* pointer to a method which already accepts them from other call sites, nothing really new being added in terms of code paths.

String changes made/needed: None
Attachment #9020507 - Flags: approval-mozilla-beta?
Comment on attachment 9020507 [details]
Bug 1497014 - Part 1: Pass in a URI hint for all content blocking state notifications

[Triage Comment]
Fixes spurious TP animations showing every time a new video segment is loaded. Approved for 64.0b6.
Attachment #9020507 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.