Closed Bug 1663992 Opened 6 months ago Closed 5 months ago

Changes in bug 1652244 break Twitter image downloads with Strict TP enabled

Categories

(Core :: Privacy: Anti-Tracking, defect, P2)

defect

Tracking

()

VERIFIED FIXED
83 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox80 --- unaffected
firefox81 blocking verified
firefox82 --- verified
firefox83 --- verified

People

(Reporter: denschub, Assigned: timhuang)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

STR:

  1. Toggle TP to Strict in the settings.
  2. Navigate to the direct URL of a Twitter image, like https://pbs.twimg.com/media/EhfHw9HXYAQ8HyW?format=png
  3. Try saving it with Ctrl/Cmd-S, or right click and "Save Image As..."

Expected:

File is downloaded somewhere.

Actual:

File fails to download, with a "Failed" in the download manager. Looking at the browser console/browser devtools, it's indicated that the request got blocked for "social tracking".


This is a regression of the changes in bug 1652244. I'm marking this as such, but since that patch fixed another broken thing, it's probably not too easy to resolve. :)

Requesting tracking just for processes sake, Twitter images appear to be working fine otherwise, so this is probably a bit niche.

I'd needinfo :dimi, but they are OOO until Sep 20. :timhuang, since you reviewed the patch, maybe you can redirect accordingly?

Flags: needinfo?(tihuang)

This is a non-default configuration and we're about to build the final beta of the 81 cycle. I don't think this needs to be tracked but I'm open to stronger arguments for why it should.

Assignee: nobody → tihuang
Severity: -- → S3
Flags: needinfo?(tihuang)
Priority: -- → P2

The problem occurs in AntiTrackingUtils::ComputeIsThirdPartyToTopWindow(). Because the channel used to download the image doesn't have a browsing context. So, AntiTrackingUtils::ComputeIsThirdPartyToTopWindow won't update the flag 'IsThirdPartyToTopWindow' correctly. I think in this case, we should check if the principal of the channel is third-party to its loading principal.

Depends on D90816

Pushed by tihuang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/71c87c0e1d24
Check loading principal if there is no browsing context in AntiTrackingUtils::ComputeIsThirdPartyToTopWindow(). r=dimi
https://hg.mozilla.org/integration/autoland/rev/0fc2e9fdf1f3
Add a test. r=dimi
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch

The patch landed in nightly and beta is affected.
:timhuang, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(tihuang)

Comment on attachment 9176815 [details]
Bug 1663992 - Check loading principal if there is no browsing context in AntiTrackingUtils::ComputeIsThirdPartyToTopWindow(). r?dimi

Beta/Release Uplift Approval Request

  • User impact if declined: The 'Save Image As' will break in image documents of social media sites when Strict mode is enabled.
  • 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 patch only changes the behavior of documents that have no browsingContext. For documents that have browsingContext, the behavior remains the same. And It matches the behavior before Bug 1652244.
  • String changes made/needed: None
Flags: needinfo?(tihuang)
Attachment #9176815 - Flags: approval-mozilla-beta?
Attachment #9176835 - Flags: approval-mozilla-beta?
Flags: in-testsuite+

Comment on attachment 9176815 [details]
Bug 1663992 - Check loading principal if there is no browsing context in AntiTrackingUtils::ComputeIsThirdPartyToTopWindow(). r?dimi

approved for 82.0b4

Attachment #9176815 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9176835 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Duplicate of this bug: 1669298

It sounds like this is the root cause behind bug 1665368 which does affect 81, so adjusting tracking flags.

Tim, can the patch in this bug be uplifted to the 81 branch to fix bug 1665368? Could you request the uplift to release? Thanks

Flags: needinfo?(tihuang)

Maybe Dimi can help here as well.

Flags: needinfo?(dlee)

Comment on attachment 9176815 [details]
Bug 1663992 - Check loading principal if there is no browsing context in AntiTrackingUtils::ComputeIsThirdPartyToTopWindow(). r?dimi

Beta/Release Uplift Approval Request

  • User impact if declined: Breakage in twitter.com when tracking protection strict mode is enabled
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: 1. First set your privacy level to “standard”
  1. Load twitter.com. Go to about:serviceworkers and confirm there’s a sw for twitter.com (via about:serviceworkers)
  2. Now change your privacy level to “strict”
  3. Go to twitter and try anything in the app that forces a full page load eg: logging out or switching accounts.
    Then you should see the error message (See Bug 1665368).

Details can be found in https://bugzilla.mozilla.org/show_bug.cgi?id=1665368#c27

  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This patch has already been tested in 82 and 83. We have seen any issues so far.
    We also have a test to cover this.
  • String changes made/needed:
Flags: needinfo?(dlee)
Attachment #9176815 - Flags: approval-mozilla-release?
Flags: qe-verify+

Comment on attachment 9176815 [details]
Bug 1663992 - Check loading principal if there is no browsing context in AntiTrackingUtils::ComputeIsThirdPartyToTopWindow(). r?dimi

Driver for 81.0.2, uplift approved for 81.0.2 thanks

Attachment #9176815 - Flags: approval-mozilla-release? → approval-mozilla-release+
Flags: needinfo?(tihuang)
QA Whiteboard: [qa-triaged]

Confirmed issue with 82.0a1 (2020-09-09) on Windows 10.
Fix verified with :dcicas using 83.0a1 (2020-10-12), 82.0b9, 81.0.2 on Windows 10, macOS 10.15, Ubuntu 16x32.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.