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 OX-6003 3rd Party Doubleclick and OpenX click tracking broken #71

Merged
merged 1 commit into from Sep 12, 2013

Conversation

Projects
None yet
2 participants
@mbeccati
Copy link
Contributor

mbeccati commented Aug 23, 2013

From https://developer.openx.org/jira/browse/OX-6003

Generating invocation tags with 3rd party click tracking enabled is broken in many ways:

  • Most of the invocation types, apart from JS, expect the clicktrack URL to be urlencoded, as the placeholder is inside an URL
  • JS invocation expects the placeholder to be replaced with an unencoded string as it performs its own encoding
  • Doubleclick's %c placeholder is replaced with an encoded string, so it works with anything but JS
  • OpenX's INSERT_ is replaced with an unencoded string, so it works only with JS

The attached patch is meant to clean up the situation a bit by:

  • Fixing JS invocation so that the placeholder is expected to be escaped
  • Fixing OpenX's 3rd party plugin so that its placeholder is replaced with {clickurl_enc}
  • Changing the generic placeholder to INSERT_ENCODED_CLICKURL_HERE, so that it's obvious what the content ought to be

I'm not sure this fix is 100% ok. Will need some thorough testing.

@andrewatfornax

This comment has been minimized.

Copy link
Contributor

andrewatfornax commented Sep 9, 2013

Looks like %c for Doubleclick is still supported, but has been updated: https://support.google.com/dfp_premium/answer/1242718?hl=en (See the "click macro" section).

Interesting though that they now have:

  • Unescaped click macro: %%CLICK_URL_UNESC%%
  • Double-escaped click macro: %%CLICK_URL_ESC_ESC%%

But these seem to be the opposite of what the above suggests %c is - is %c simply single escaped? Is there now no option to get just the single escaped URL from Doubleclick? Or is %c "double escaped" and that is what is required?

@andrewatfornax

This comment has been minimized.

Copy link
Contributor

andrewatfornax commented Sep 9, 2013

So the changes all make sense to me. I think the think that is worth nothing though is that existing "OpenX zone in another OpenX as the 3rd party" setups are going to break. That is, at the moment:

  • An OpenX Javascript zone tag that's being used as an OpenX creative WILL work, as JS expects an unescaped ct0 URL, and that is what is used; and
  • All other OpenX zone tags being used as an OpenX creative will NOT work, as they expect an escaped ct0 URL, and that is not what is being used.

If the OpenX instance that provided the zone tags is upgraded to Revive Adserver (once the above change is merged), then nothing would change - unless the zone tags were updated. In that case, then:

  • The new Revive Javascript zone tag now used in an old OpenX setup as a creative will NOT work, as the JS tag would expect an escaped ct0 URL, but the old OpenX setup would provide it unescaped; and
  • All new other new Revive zone tags now used in an old OpenX setup as creatives will continue to not work, as they continue to expect an escaped ct0 URL, which the old OpenX would continue to provide unescaped.

So, using new Revive Adserver tags in an old OpenX installation, if the changes were merged, would break all cases.

However, if the OpenX instance that is acting as the 3rd party server is upgraded to Revive Adserver (once the above change is merged), then things would change instantly:

  • The old OpenX Javascript zone tag being used in the new Revive Adserver 3rd party server will NOT now work, as the old tag expects an unescaped ct0 URL, but the new 3rd party server will provide it escaped; and
  • All other old OpenX zone tags being used in the new Revive Adserver 3rd part server WILL now work, as the old tags expected escaped ct0 URLs, and that is now what the 3rd party server will provide.

Of course, if BOTH servers are updated, and the zone tags are all updated, then everything will be using escaped URLs, so everything will work.

The upshot of all of this is, that I believe that merging these changes:

  1. Will not affect users who have put their zone tags into a 3rd party OpenX installation if they don't update the creatives with the 3rd party - what is broken now will continue to be broken.
  2. Will break users who upgrade to Revive Adserver and then pass over new zone tags to a 3rd party OpenX installation. New Revive Adserver tags, of all types, won't click track correctly in old OpenX installations.
  3. Will break 3rd party OpenX installations who upgrade. OpenX installations that upgrade to Revive Adserver and who are acting as a 3rd party server for old OpenX tags will stop click tracking for existing working JS tags (although all other tags that previously did not track, will start working).

So, realistically, while this change makes sense, it will break OpenX/Revive and Revive/OpenX tracking. Users of both installations would realistically need to upgrade, and all tags be re-issued, for the changes to be fully in place.

I don't think that is necessarily a bad thing, as it does sort out the mess, but we would have to be clear about it in the release notes - and a major version number is a good time to do it.

andrewatfornax added a commit that referenced this pull request Sep 12, 2013

Merge pull request #71 from openx/ox-6003-doubleclick
Fix OX-6003 3rd Party Doubleclick and OpenX click tracking broken

@andrewatfornax andrewatfornax merged commit a8ad15b into master Sep 12, 2013

andrewatfornax added a commit that referenced this pull request Sep 12, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment