Skip to content

ssr intrinsic layout: support svg using apostroph#26294

Merged
sebastianbenz merged 1 commit intoampproject:masterfrom
sebastianbenz:ssr-intrinsic2
Mar 2, 2020
Merged

ssr intrinsic layout: support svg using apostroph#26294
sebastianbenz merged 1 commit intoampproject:masterfrom
sebastianbenz:ssr-intrinsic2

Conversation

@sebastianbenz
Copy link
Copy Markdown
Contributor

Intrinsic layout validation originally required inline SVG attribute quotes to be escaped via ". To reduce char count, this PR changes the behavior to expect single quotes instead.

This is a breaking change, however, I don't think that this will be an issue as intrinsic layout SSR hasn't been implemented anywhere. If this is a concern, we could change the implementation to support both.

//cc @jridgewell

@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Jan 10, 2020

Hey @ampproject/wg-caching, these files were changed:

validator/testdata/transformed_feature_tests/server_side_rendering.html
validator/testdata/transformed_feature_tests/server_side_rendering.out
validator/validator-main.protoascii

@sebastianbenz sebastianbenz changed the title 🐛require inline svg to use single quotes 🐛SSR: require inline svg to use single quotes Jan 10, 2020
@twifkak twifkak requested a review from Gregable January 13, 2020 21:01
@Gregable Gregable removed the request for review from honeybadgerdontcare January 13, 2020 22:40
@sebastianbenz sebastianbenz changed the title 🐛SSR: require inline svg to use single quotes ssr intrinsic layout: support svg using apostroph Feb 27, 2020
@sebastianbenz
Copy link
Copy Markdown
Contributor Author

If changed the implementation to support both, escaped quotes or single quotes (before only escaped quotes were valid).

This means, these two SVG placeholders will be valid:

data:image/svg+xml;charset=utf-8,<svg height=&quot;100&quot; width=&quot;300&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot; version=&quot;1.1&quot;/>

data:image/svg+xml;charset=utf-8,<svg height='100' width='300' xmlns='http://www.w3.org/2000/svg' version='1.1'/>

Reason for this change: in the meantime intrinsic support has been added to AMP Optimizer which has been using escaped quotes.

@sebastianbenz
Copy link
Copy Markdown
Contributor Author

@honeybadgerdontcare @Gregable PTAL

@sebastianbenz
Copy link
Copy Markdown
Contributor Author

Is there a way to re-trigger the build?

This change updates the validatation of the intrinsic SVG placeholder to
support either escaped quotes or single quotes (before only escaped
quotes were valid).

This means, these two SVG placeholders will be valid:

```
data:image/svg+xml;charset=utf-8,<svg height=&quot;100&quot; width=&quot;300&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot; version=&quot;1.1&quot;/>

data:image/svg+xml;charset=utf-8,<svg height='100' width='300' xmlns='http://www.w3.org/2000/svg' version='1.1'/>
```
@sebastianbenz
Copy link
Copy Markdown
Contributor Author

Re-based against master to re-trigger the build.

@sebastianbenz sebastianbenz merged commit aaba384 into ampproject:master Mar 2, 2020
@sebastianbenz sebastianbenz deleted the ssr-intrinsic2 branch March 2, 2020 12:55
robinvanopstal added a commit to jungvonmatt/amphtml that referenced this pull request Mar 2, 2020
* master:
  ssr intrinsic layout: support svg using apostroph (ampproject#26294)
Gregable pushed a commit that referenced this pull request Mar 3, 2020
* cl/297284764 n/a

* cl/298442636 Revision bump for #26841

* cl/298451814 Revision bump for #26294

* fix merge duplication

Co-authored-by: Amaltas <amaltas@amaltas.org>
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.

5 participants