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

Analytics Type lowercases on save #1133

Closed
robertsky opened this Issue May 8, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@robertsky
Copy link

robertsky commented May 8, 2018

Issue: Upon saving, the Analytics Type indicated is saved with the string in lower case. This leads to the config for the type not detected.
Example: adobeanalytics_nativeConfig
On saving: adobeanalytics_nativeconfig

My current workaround is to use the amp_post_template_analytics filter instead. However, this issue might lead others to wonder why their analytics tag is not working properly.

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented May 9, 2018

Can you share the full config you are using?

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented May 11, 2018

Nevermind. I see it. The problem is that sanitize_key() normalizes by lower-casing.

https://github.com/Automattic/amp-wp/blob/d2b2c2dccc7a6d4ddd0a2e27bb5715c36dc33f45/includes/options/class-amp-options-manager.php#L127-L131

I suggest replacing the instances of sanitize_key( $x ) with preg_replace( '/[^a-z0-9_\-]/', '', $x ).

@robertsky

This comment has been minimized.

Copy link
Author

robertsky commented May 15, 2018

Apologies for not responding sooner. Glad that you have found the cause! I hope the fix would be in the codebase sometime for other users!

@westonruter westonruter added this to the v1.0 milestone May 21, 2018

@westonruter westonruter removed this from the v1.0 milestone May 30, 2018

AdelDima added a commit to AdelDima/amp-wp that referenced this issue Jul 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.