Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Remove Yandex' trackers from URLs" #21175

Merged
merged 1 commit into from
Dec 4, 2023
Merged

Revert "Remove Yandex' trackers from URLs" #21175

merged 1 commit into from
Dec 4, 2023

Conversation

fmarier
Copy link
Member

@fmarier fmarier commented Dec 1, 2023

This reverts commit fca0aa8.

Related to brave/brave-browser#33216. Will re-add once brave/brave-browser#10188 is fixed.

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

  1. Open https://brave.com/?ymclid=1234&ysclid=abcde.
  2. Confirm that the URL bar shows the full URL (no parameters were removed).

@fmarier fmarier self-assigned this Dec 1, 2023
Copy link
Contributor

github-actions bot commented Dec 1, 2023

[puLL-Merge] - brave/brave-core@21175

The text you've provided is a description of a commit in what appears to be a Git repository. This commit message indicates that the author, Francois Marier from Brave (an internet browser company), has made a patch to revert a previous commit. A "revert" in this context means undoing the changes that were made in a previous commit.

The content of the patch shows changes made to a single file: components/query_filter/utils.cc.

Here's a breakdown of what the patch does:

  • The commit message includes the following:

    • The author of the change.
    • The date of the change.
    • A subject line describing the patch — specifically, that it's reverting a prior change related to removing Yandex trackers from URLs. This likely refers to an issue with an ID of brave/brave-browser#33216 in the Brave browser's issue tracking system. Yandex is a Russian multinational corporation that provides a range of internet-related products and services, including a search engine that uses such trackers.
  • The patch includes a diff section that shows the before-and-after state of the code:

    • The --- line shows the original file, while the +++ line shows the new state of the file after the changes are applied.
    • The @@ -102,9 +102,6 @@ indicates the context of changes in the file, meaning that the line numbers of the file in the old version (from -102) and the new version (to +102) and the number of lines that the diff context includes.

-The actual change shows that three lines (with comments indicating Yandex's ymclid and ysclid trackers) are removed (-). These lines were part of a list of what appears to be tracker parameters that would be stripped from URL query strings. Since this is a revert, those lines were in the original code base, then removed by the previous commit that is now being reverted, which means they will now be part of the code base again after this patch is applied.

To summarize, this patch is undoing a previous change that removed two Yandex tracking parameters (ymclid and ysclid) from a list that the Brave browser uses to filter out tracking information from URLs. After this patch, the Brave browser will no longer remove these specific tracking parameters from Yandex URLs.

@fmarier fmarier marked this pull request as ready for review December 4, 2023 21:35
Copy link
Collaborator

@cdesouza-chromium cdesouza-chromium left a comment

Choose a reason for hiding this comment

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

LGTM

@fmarier fmarier merged commit 700a2d5 into master Dec 4, 2023
23 checks passed
@fmarier fmarier deleted the revert-33216 branch December 4, 2023 22:16
@github-actions github-actions bot added this to the 1.63.x - Nightly milestone Dec 4, 2023
brave-builds added a commit that referenced this pull request Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants