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

Consider supporting a list of url in attributionsrc on anchors/window.open #718

Closed
johnivdel opened this issue Mar 6, 2023 · 9 comments · Fixed by #744
Closed

Consider supporting a list of url in attributionsrc on anchors/window.open #718

johnivdel opened this issue Mar 6, 2023 · 9 comments · Fixed by #744

Comments

@johnivdel
Copy link
Collaborator

This would be useful for cases similar to #696, where only the first adtech knows the other reporting origins which may want to register for a specific click.

Moving these flows into a redirect chain involves changes from all reporting origins to perform a 302 redirect, rather than just the initial reporting origin making a change.

Note that today the ping attribute supports a space separated list of urls, which behaves similarly to the attributionsrc attribute in terms of sending background requests.

@csharrison
Copy link
Collaborator

csharrison commented Mar 6, 2023

I think this would solve the documented gap from VAST: #653

@apasel422
Copy link
Collaborator

The issue title mentions anchors and window.open. Is there a reason to omit support for this on <img> and <script>?

@jfggit
Copy link

jfggit commented Mar 8, 2023

I think this would be a valuable addition to the API and would solve some use cases (or at least make them less awkward to handle). I think we'd probably want to be careful about how we list-ify attributionsrc. Presumably we'd want to keep the existing single URL usage consistent, but supporting say, comma-separated lists, might be less safe than it seems. I vaguely recall seeing unescaped commas in URLs in the past which could end up being problematic. Either way, supportive of this in general and +1 to supporting on <img> and <script> as well.

As for solving the gap from #653, I'm not sure if that's right. The gap there specifically is that there is no way to register a non-navigation ping as a navigation-type registration source event. So in VAST, there is one clickthrough and potentially many click trackers, where the formers is the actual navigation on ad click and the latter are just sent as separate pings. The gap is that the latter are then interpreted as "view" type event sources, whereas they're technically recording navigation events.

@csharrison
Copy link
Collaborator

As for solving the gap from #653, I'm not sure if that's right. The gap there specifically is that there is no way to register a non-navigation ping as a navigation-type registration source event. So in VAST, there is one clickthrough and potentially many click trackers, where the formers is the actual navigation on ad click and the latter are just sent as separate pings. The gap is that the latter are then interpreted as "view" type event sources, whereas they're technically recording navigation events.

I was assuming the VAST SDK would accumulate all of the "click tracker" URLs and join like this:

<a href="<VAST click through URL>" attributionsrc="<space separated VAST click trackers>">clickme</a>

In this way, each click tracker would get a separate ping, and it would be classified as a click rather than a view.

@jfggit
Copy link

jfggit commented Mar 8, 2023

Ah, yeah, that makes sense. And I guess it would work the same way for window.open then? In which case, yes, I think that does probably mostly solve the issue.

@csharrison
Copy link
Collaborator

Yep, this issue covers window.open, we'd want to keep them consistent. Thanks for confirming @jfggit .

@johnivdel
Copy link
Collaborator Author

johnivdel commented Mar 23, 2023

@apasel422 pointed out that integrating a url list with window.open is not striaghtforward.

Using a space separated list in the attributionsrc attribute aligns with <a> ping

Because attributionsrc is included as a feature in the window.open features param, a space separated list will not work.

https://html.spec.whatwg.org/multipage/nav-history-apis.html#concept-window-open-features-tokenize

As the space is used as a feature token separator already in the spec.

The attributionsrc feature is a url encdoded string today. A few options seem like:

  1. Use a special separator not reserved by https://html.spec.whatwg.org/multipage/nav-history-apis.html#feature-separator, and also guaranteed to be % encoded. For example: @ | ^ /
window.open(url, "_top", "attributionsrc=https%3A%2F%2Fa.example|https%3A%2F%2Fb.example")
  1. Monkeypatch the tokenize features algorithm to support multiple separate definitions of some features s.t. it builds a list:
window.open(url, "_top", "attributionsrc=https%3A%2F%2Fa.example attributionsrc=https%3A%2F%2Fb.example")
  1. Something else?

@domenic for thoughts here

@domenic
Copy link

domenic commented Mar 24, 2023

I mean, stuffing all this into a string really sucks, right? I think in the ideal world, someone would work on whatwg/html#7485 first and then we'd have a clear extensibility point. I don't know if that's something you're interested in, but I hear @annevk is somewhat supportive of the idea as well, so it might be doable.

Otherwise, if you're going to stick with the string syntax, a few considerations:

  • The main consideration is making sure that what you use here doesn't break other browsers that don't implement this new algorithm. So, I think your (1) meets that. (2) is less obvious but I think it's also fine.

  • There is never a guarantee that web developers will properly percent-encode the values they pass you. You need a processing model and tests for the edge cases. E.g. what happens if they pass the JavaScript string "attributionsrc=https://a.example|https://b.example". Does it work? Probably, if you're using the processing model "split on |, percent-decode, then URL parse". Is the result what the web developer expected? I don't know, maybe??

Between your (1) and (2) I don't have a strong preference; everything here is a mess and will hopefully be obsoleted one day by a better API.

@apasel422
Copy link
Collaborator

The issue title mentions anchors and window.open. Is there a reason to omit support for this on <img> and <script>?

Following up on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants