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

stop data attributes from being stripped #9501

Merged
merged 1 commit into from Mar 16, 2018
Merged

stop data attributes from being stripped #9501

merged 1 commit into from Mar 16, 2018

Conversation

CriticalRespawn
Copy link
Contributor

@CriticalRespawn CriticalRespawn commented Mar 15, 2018

This PR (hopefully) fixes an issue raised in #9500 where data-* attributes get stripped from the HTML on AMP pages.

This PR simply adds data-* to the allowedAMPAttributes array.

Copy link
Member

@aileen aileen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @CriticalRespawn!

The changes look fine and also work as expected. 👍

If you want to look into doing more PRs for Ghost, you can check out our guidelines and docs: https://docs.ghost.org/docs/contributing#section-submitting-pull-requests . They helped me a lot! 😉

@aileen
Copy link
Member

aileen commented Mar 15, 2018

@kirrg001 I don't know why this is happening here, but Node 4 seems to have a problem with knex-migrator: https://travis-ci.org/TryGhost/Ghost/jobs/353644294#L764 and therefore Travis is failing.

I was facing the same problems locally, when I tried to debug this. I couldn't even run knex-migrator init.

@kirrg001
Copy link
Contributor

@AileenCGN Why do you think it's knex-migrator? 🤔

SyntaxError: Unexpected token {
at exports.runInThisContext (vm.js:53:16)
at require (internal/module.js:12:17)
at Object. (core/server/lib/mobiledoc/cards/index.js:2:12)

@aileen
Copy link
Member

aileen commented Mar 16, 2018

Why do you think it's knex-migrator?

Because it happened as well locally when I tried to reproduce this in Node 4 and knex-migrator init failed for the same reason. The error log looked a bit different (still Unexpected token, but failing in knex-migrator).

@aileen aileen merged commit 735d977 into TryGhost:master Mar 16, 2018
@CriticalRespawn CriticalRespawn deleted the dont-strip-data-attributes branch March 23, 2018 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants