Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Update TagBuilder to support valueless attributes. #4079

Closed
wants to merge 1 commit into from
Closed

Update TagBuilder to support valueless attributes. #4079

wants to merge 1 commit into from

Conversation

geertdoornbos
Copy link

Ref. #4078

This change makes it possible to add valueless attributes. For instance, the async attribute: <script src="..." async>

Attributes can now be rendered in the three ways, specified by w3c: http://www.w3.org/TR/html5/infrastructure.html#boolean-attribute

@geertdoornbos
Update TagBuilder to support valueless attributes.
9b886ca 
This change makes it possible to add valueless attributes. For instance, the async attribute: <script src="..." async>

Attributes can now be rendered in the three ways, specified by w3c: http://www.w3.org/TR/html5/infrastructure.html#boolean-attribute
@dnfclas
Copy link

dnfclas commented Feb 11, 2016

Hi @geertdoornbos, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, DNFBOT;

@rynowak
Copy link
Member

rynowak commented Feb 11, 2016

This change looks good, needs a unit test though. Similar to https://github.com/aspnet/Mvc/blob/dev/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/TagBuilderTest.cs#L117

@dougbu
Copy link
Member

dougbu commented Feb 11, 2016

If we did this (and something was proposed offline ages ago and shot down), we can't do it this way. null is a valid attribute value and it results in name="" which is semantically different from name or name="name". So this PR would result in a breaking change.

@geertdoornbos
Copy link
Author

Don't want a breaking change of course. Thanks for the quick feedback.
I didn't know TagBuilder is bound to XHTML syntax.
Ah semantics, if someone passes a null value for a string type attribute and generates code; the parser could see it as a boolean type attribute.

But it's 2016 now 😄 time to leave TagBuilder behind I guess, if it can't support html5 style.
Thanks!

@benaadams
Copy link
Contributor

Switch on if <!DOCTYPE html> is at top of page?

@geertdoornbos
Copy link
Author

@benaadams I don't think there is a good way to do that inside TagBuilder. Altering TagBuilder to support html5 style is also not a good plan. Thanks anyway.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@geertdoornbos @dnfclas @rynowak @dougbu @benaadams