Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.

Conversation

@ladamski
Copy link
Collaborator

@ladamski ladamski commented Aug 10, 2023

Task/Issue URL: https://app.asana.com/0/1201037661562251/1205194870935032/f
Tech Design URL:
CC:

Description:
Disable CTL for Catalina (and earlier)
Steps to test this PR:
Visit https://privacy-test-pages.glitch.me/privacy-protections/click-to-load/

  1. For MacOS 11+, CTL should work as normal
  2. For Catalina, it should be disabled and all FB buttons and elements should load (for SDK and iframe elements)
  3. Toggle protections in each and ensure behavior is as expected (MacOS 11+ CTL is enabled and disabled, for Catalina it remains disabled regardless)

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

@samsymons
Copy link
Collaborator

samsymons commented Aug 10, 2023

@tomasstrba How far away is the macOS Catalina removal work? We might be able to supersede this PR with that

@tomasstrba tomasstrba self-requested a review August 17, 2023 11:23
@tomasstrba tomasstrba self-assigned this Aug 17, 2023
Copy link
Contributor

@tomasstrba tomasstrba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ladamski, I am getting syntax and type errors in the Console for changes suggested for Catalina. Can you reproduce it?

Screenshot 2023-08-17 at 1 46 14 PM

@tomasstrba tomasstrba assigned ladamski and unassigned tomasstrba Aug 17, 2023
@ladamski
Copy link
Collaborator Author

ladamski commented Aug 17, 2023

The type error is expected as we're passing in an empty FB config so it doesn't parse gracefully. Since its just a quick fix for Catalina I think that's ok, since the content script just bails (and it shouldn't be happening for non-Catalina users).

The syntax error I can't reproduce. I'm actually not sure what sdk.js is. Is this on the stock branch or one you tweaked to test? Or which page are you testing on?

@ladamski, I am getting syntax and type errors in the Console for changes suggested for Catalina. Can you reproduce it?

Copy link
Contributor

@tomasstrba tomasstrba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lucas, I investigated the syntax error and found out it was the autoconsent script. We had an issue with it last week which probably still exist in this branch. After merging develop into the branch, the syntax error is no longer there.

LGTM! ✅

@tomasstrba tomasstrba mentioned this pull request Aug 18, 2023
@ladamski ladamski merged commit 4258c30 into develop Aug 25, 2023
@ladamski ladamski deleted the la/disable-CTL-for-Catalina branch August 25, 2023 10:11
samsymons added a commit that referenced this pull request Aug 28, 2023
# By Diego Rey Mendez (3) and others
# Via GitHub
* develop:
  NetP menu no longer opening (#1552)
  Adjusts the NetP onboarding bg color (#1550)
  Save/edit always the same profile (#1545)
  Enable autoconsent tests for cosmetic rules (#1473)
  Title doubleclick action (#1549)
  NetP onboarding: changes a color and updates some assets (#1547)
  La/disable ctl for catalina (#1462)
  fix tab content disappearance due to wrong pinned tab index selection (#1532)
  disable flaky test (#1546)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants