Skip to content

Nested macro bug #20816

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

Closed
zhouyx opened this issue Feb 12, 2019 · 3 comments · Fixed by #22508
Closed

Nested macro bug #20816

zhouyx opened this issue Feb 12, 2019 · 3 comments · Fixed by #22508

Comments

@zhouyx
Copy link
Contributor

zhouyx commented Feb 12, 2019

cc @ampproject/wg-analytics

In the following example, nested macro doesn't work properly

$REPLACE(CLIENT_ID(navigationStart)test, 'AD_', '')"
resolves to client_id, note that test is dropped

$REPLACE(testCLIENT_ID(navigationStart), 'AD_', '')"
resolves to test, note that client_id is dropped

$REPLACE(NAV_TIMING(navigationStart, loadEventStart), 'AD_', '')"
resolves to empty string, note that the nav_timing number is dropped.

I believe there are two issues here

  1. Nested macro doesn't handle number properly
  2. Nested macro doesn't handle concat string properly
@calebcordry
Copy link
Member

:( Thanks for reporting.

@ampprojectbot ampprojectbot added this to the Backlog Bugs milestone Feb 13, 2019
@ampprojectbot
Copy link
Member

This issue doesn't have a category which makes it harder for us to keep track of it. @calebcordry Please add an appropriate category.

@zhouyx
Copy link
Contributor Author

zhouyx commented May 23, 2019

Reported by @zikas :

I found the bug to be related to the issue here.

$IF(QUERY_PARAM(test), 1.$SUBSTR(TIMESTAMP, 0, 10)QUERY_PARAM(test), '')
resolve to 1.

and

$IF(QUERY_PARAM(gclid), $SUBSTR(TIMESTAMP, 0, 10)QUERY_PARAM(gclid), '')
resolve to 1234567890 (equals to $SUBSTR(TIMESTAMP,0,10)))

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

Successfully merging a pull request may close this issue.

3 participants