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

Don't close <img> tag in HTML #514

Merged
merged 1 commit into from
Oct 24, 2017

Conversation

dereckson
Copy link
Contributor

Reference: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img

"Tag omission: Must have a start tag and must not have an end tag."

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 23, 2017
@SendGridDX
Copy link

SendGridDX commented Oct 23, 2017

CLA assistant check
All committers have signed the CLA.

@@ -57,7 +57,7 @@
"content": [
{
"type": "text/html",
"value": "<html><p>Hello, world!</p><img src=[CID GOES HERE]></img></html>"
"value": "<html><p>Hello, world!</p><img src=[CID GOES HERE]></html>"

Choose a reason for hiding this comment

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

Should it have an internal forward slash, like so:

      "value": "<html><p>Hello, world!</p><img src=[CID GOES HERE] /></html>"

Choose a reason for hiding this comment

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

xhtmlish yes, html5 not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/ are actually invalid, not facultative, in HTML 5. It's an XML only feature, and so only used on the old XHTML standard.

@@ -1096,7 +1096,7 @@ public function test_mail_send_post()
"content": [
{
"type": "text/html",
"value": "<html><p>Hello, world!</p><img src=[CID GOES HERE]></img></html>"
"value": "<html><p>Hello, world!</p><img src=[CID GOES HERE]></html>"

Choose a reason for hiding this comment

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

Should it have an internal forward slash, like so:

      "value": "<html><p>Hello, world!</p><img src=[CID GOES HERE] /></html>"

@mbernier mbernier added difficulty: easy fix is easy in difficulty hacktoberfest labels Oct 23, 2017
Reference: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img

"Tag omission: Must have a start tag and must not have an end tag."
@dereckson dereckson force-pushed the redundant-html-closing-tags branch from fbdd946 to 87026f8 Compare October 23, 2017 20:39
@mbernier mbernier merged commit 8ba68fe into sendgrid:master Oct 24, 2017
@thinkingserious
Copy link
Contributor

Hello @dereckson,

Thanks again for the PR!

We want to show our appreciation by sending you some swag. Could you please fill out this form so we can send it to you? Thanks!

Team SendGrid DX

@dereckson dereckson deleted the redundant-html-closing-tags branch October 24, 2017 14:22
@dereckson
Copy link
Contributor Author

You're welcome. Thanks for the swag.

@mbernier
Copy link

@dereckson
If you are interested, we would love the opportunity to talk to you about Hacktoberfest and our API libraries.

Our agenda would be: Explore what you liked and is there anything we can do to improve?

You can grab a time on my calendar that works for you and we can have a chat on Google Hangout or Skype. If you prefer, you can email me using my GitHub username at my company’s domain.

Thank you so much,

Matt Bernier - @mbernier - SendGrid Developer Experience Product Manager
Elmer Thomas - @thinkingserious - SendGrid Developer Experience Engineer
@sendgrid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy fix is easy in difficulty status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants