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

Fix verifiable ad conversion idiosyncrasies #16049

Closed
iambrianfung opened this issue May 24, 2021 · 3 comments
Closed

Fix verifiable ad conversion idiosyncrasies #16049

iambrianfung opened this issue May 24, 2021 · 3 comments
Assignees

Comments

@iambrianfung
Copy link
Collaborator

Misc edge cases that can break the VAC feature.

  1. mismatched conversion URLs

    1. the conversion URLs in the catalog & resource file must match.
      • ex. for conversion URL https://brave.com/careers/
        • https://brave.com/careers/* + https://brave.com/careers* will fail to fetch the conversion id
        • https://brave.com/careers/* in both or https://brave.com/careers* in both will do so successfully.
  2. do not use single-quotes for HTML attributes in the regex.

    1. Brave browser re-interprets single-quotes in HTML as double-quotes in the DOM.
      click to expand
      single-quotes
    2. using single-quotes in the regex for the VAC serializer will fail to capture the conversion id.
@iambrianfung iambrianfung added OS/Android Fixes related to Android browser functionality OS/Desktop feature/ads labels May 24, 2021
@tmancey tmancey added the bug label Jul 26, 2021
@tmancey tmancey removed the OS/Android Fixes related to Android browser functionality label Jul 26, 2021
@tmancey tmancey changed the title Verifiable Ad Conversions Idiosyncrasies Fix verifiable ad conversion idiosyncrasies Apr 13, 2023
@tmancey tmancey added priority/P3 The next thing for us to work on. It'll ride the trains. QA/Yes release-notes/exclude labels Apr 13, 2023
@tmancey tmancey self-assigned this May 23, 2023
@tmancey tmancey added enhancement and removed bug labels Jun 22, 2023
@tmancey
Copy link
Contributor

tmancey commented Jul 10, 2023

@iambrianfung as discussed in DM #1 will be resolved by transitioning conversions from ads resources to the catalog. Lets raise a new issue and only resolve #2 for this ticket.

@tmancey
Copy link
Contributor

tmancey commented Jul 10, 2023

With regards to #2 are we saying we should try replace ` with " before trying the match?

@tmancey
Copy link
Contributor

tmancey commented Jul 11, 2023

@tackley discussed with @iambrianfung and we will raise issues to normalize regex, i.e. replace single quotes with double quotes in the resource generator for #2.

#1 will be resolved when transitioning conversions from component updater resources to the catalog.

@tmancey tmancey closed this as completed Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

2 participants