-
Notifications
You must be signed in to change notification settings - Fork 173
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
Impression declaration for links opened through Window.open #77
Comments
Supporting JS navigation APIs seems reasonable, especially in the context of #22 and privacycg/private-click-measurement#31, which discuss adding JS support for conversion registration. A few directions we could go to support window.open navigations are: Supply impression registration attributes directly to the window.open() callAdd a new param to window.open which takes an impression for the navigation.
where ImpressionInfo is a new IDL dictionary:
One downside to this approach is that it does not work for other JS Navigation API's such as window.location.assign(), and params would need to be added to support those navigations as well. Add a separate API to associate navigation urls with impression attributesAdd an API surface which associates an
Example usage:
This would allow impressions to be registered for any navigation, including window.open and navigations from anchor tags. Impression registered in this manner would not make |
cc @csharrison for thoughts |
I like this idea John! Let me cc @domenic for thoughts as well. Some initial thoughts
This way it would be trivial to add other extensions if we wanted to match on other things like domains, origins, etc. |
A global map that only impacts the next call to specific APIs is a bit strange. It's like if your C++ code, instead of calling a function by passing arguments, set a global variable and then expected the function to look up its input data in the global variable. Generally this is discouraged in most codebases. Another big thing to consider on global-map vs. per-call-site is whether it's OK for any script running in the window to modify the impression mapping. E.g., if I include Overall, to me added arguments to |
Oh I saw this as a permanent map not one that just impacts the next call. I see your point about it being strange and I agree it is non-ideal. The benefit in my mind is just a single interface to use the API rather than changing all possible API surfaces for navigation.
I think this is reasonable but to be fair can't all those JS APIs be shimmed and mutated as well if they are running in the same context?
Thanks, I appreciate this feedback. I'm happy to go with that option, especially if we can just use the same init struct in all cases. @johnivdel what do you think? |
Good point! That pretty much invalidates that paragraph of my response :) |
Discussed offline with domenic and johnivdel. We all agreed that adding direct support to window.open, location.assign, and possibly some new API on the anchor element was the best path forward. Those APIs could all take the same init struct to configure the API. An execution-scoped map has too many downsides (action at a distance, etc) to be worth it to remove an argument to a few call sites. |
Adds a section denoting how to register impressions for window.open navigations per #77
Is there anything left to do here? As far as I know, we support impression declaration via |
Closing as this was superseded by #130 |
The API currently only specifies implementation for links opened through HTML anchor tags, but we also have links that are programmatically navigated through Window.open. What, if any, is the appropriate way to do impression declaration for these?
The text was updated successfully, but these errors were encountered: