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 remove meaningful empty elements #538

Merged
merged 3 commits into from
Mar 6, 2016
Merged

don't remove meaningful empty elements #538

merged 3 commits into from
Mar 6, 2016

Conversation

alexlamsl
Copy link
Collaborator

#536 mentions <iframe> - I've further identified <audio> and <video>.

case 'textarea':
case 'video':
return false;
case 'script':
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't video, audio and iframe act the same way as script? Only be removed when they don't have "src" attr?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was being conservative - but yeah let's go with yours.

Can you think of other tags while we are at this?

@alexlamsl
Copy link
Collaborator Author

@kangax commit amended

@kangax
Copy link
Owner

kangax commented Mar 6, 2016

Perhaps <object>?

@alexlamsl
Copy link
Collaborator Author

Assuming <object data="..."> then?

@alexlamsl
Copy link
Collaborator Author

Right, added <applet code="..."> while we are at this.

@alexlamsl
Copy link
Collaborator Author

... I've just discovered <spacer> as well 😓

@alexlamsl
Copy link
Collaborator Author

Okay, looks like only Netscape and very old Mozilla browers support <spacer>, so I guess it's not a big deal...?

@kangax
Copy link
Owner

kangax commented Mar 6, 2016

HTML is hard :D

On Sun, Mar 6, 2016 at 5:28 PM, Alex Lam S.L. notifications@github.com
wrote:

... I've just discovered as well [image: 😓]


Reply to this email directly or view it on GitHub
#538 (comment).

@alexlamsl
Copy link
Collaborator Author

I'm actually thinking about <i class="fa fa-camera-retro"> and same with <span> - they are commonly used to put out these font-based icons and removeEmptyElements would kill them.

@kangax
Copy link
Owner

kangax commented Mar 6, 2016

Not sure it's worth adding support for spacer. It's deprecated, no?

@alexlamsl
Copy link
Collaborator Author

Probably not - it's one of those "no harm, let's throw in the kitchen sink" moment 👻

I'll take it back out then...

@alexlamsl
Copy link
Collaborator Author

So what are your thoughts on sparing <i> and <span> when class attribute is defined?

@kangax
Copy link
Owner

kangax commented Mar 6, 2016

So what are your thoughts on sparing and when class attribute is defined?

That's expected. We should make it very clear in the docs. Ideally, they need to be ignored via config or something.

@alexlamsl
Copy link
Collaborator Author

I guess that link to your blog post has enough warning signs on it.

@alexlamsl
Copy link
Collaborator Author

<spacer> commit removed 😉

@kangax
Copy link
Owner

kangax commented Mar 6, 2016

👍

kangax added a commit that referenced this pull request Mar 6, 2016
don't remove meaningful empty elements
@kangax kangax merged commit af26ab3 into kangax:gh-pages Mar 6, 2016
@alexlamsl alexlamsl mentioned this pull request Mar 8, 2016
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