Skip to content

Clean up inconsistent script tags in AMP .md files#9557

Closed
rsimha wants to merge 1 commit intoampproject:masterfrom
rsimha:2017-05-25-CleanUpScriptTags
Closed

Clean up inconsistent script tags in AMP .md files#9557
rsimha wants to merge 1 commit intoampproject:masterfrom
rsimha:2017-05-25-CleanUpScriptTags

Conversation

@rsimha
Copy link
Copy Markdown
Contributor

@rsimha rsimha commented May 25, 2017

Several AMP .md files have inconsistent script tags. Some have escaped "& lt;" tags paired with unescaped > tags.

This PR replaces all escaped script tags in .md files with their unescaped equivalents.

Fixes #9559

Copy link
Copy Markdown
Member

@cramforce cramforce left a comment

Choose a reason for hiding this comment

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

As discussed in other PR, this is incorrect. These are docs embedded in HTML. They need to be escaped.

@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented May 25, 2017

Continuing discussion about inconsistent tags in this PR.

@cramforce, @pbakaus: Sounds good. I'll abandon this change.

I wonder though, if it's worth making the braces consistent and escaped (instead of unescaped), since there are currently many instances of opening braces being escaped while their corresponding close braces not being escaped.

@pbakaus
Copy link
Copy Markdown
Contributor

pbakaus commented May 25, 2017

@rsimha-amp definitely in favor of consistency. To summarize:

  1. unescaping in markdown blocks should be OK
  2. unescaping in <code> blocks (or other arbitrary HTML) will break us.

@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented May 26, 2017

@pbakaus, thanks. Will send you a new PR with fixes based on your guidance.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants