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

Editor template assumes all IHtmlContent implementations override ToString() #5317

Closed
dougbu opened this issue Sep 25, 2016 · 2 comments
Closed
Assignees

Comments

@dougbu
Copy link
Member

dougbu commented Sep 25, 2016

The default HTML template for a POCO object examines the IHtmlHelper.Label() return value using !string.IsNullOrEmpty(label.ToString()). This condition assumes all IHtmlContent implementations override ToString(). Further the condition works only as long as ToString() returns string.Empty when the IHtmlContent implementation will no-op in its WriteTo() method.

This problem does not show symptoms when using the default Label() implementation. That method always returns either HtmlString.Empty or a TagBuilder. HtmlString.Empty.ToString() returns the empty string. TagBuilder.ToString() always returns the type's FullName and TagBuilder.WriteTo() always writes something - at least angle brackets and the tag name. So, things appear to work at the moment.

Example cases where the current assumptions would be invalid:

  • We update TagBuilder to have a new TagRenderMode.Empty but still do not override ToString().
  • In 2.0, we remove the HtmlString.ToString() override because it's redundant with HtmlString.Value and maintained only for back-compat.
  • Users subclass HtmlHelper and override the GenerateLabel() method to return their own IHtmlContent implementation. That implementation does not satisfy our assumptions. (The assumptions aren't contractual or enforced of course.)

Side note: The default IHtmlHelper.Label() implementation rarely returns HtmlString.Empty but it definitely can do so. E.g. a property may have [Display(Name = "")].

@dougbu dougbu added this to the 1.1.0 milestone Sep 25, 2016
@dougbu dougbu self-assigned this Sep 25, 2016
@dougbu
Copy link
Member Author

dougbu commented Sep 25, 2016

Taking because I've got the fix and previously discussed the issue w/ @Eilon

dougbu added a commit that referenced this issue Sep 25, 2016
- #5317
- worked only because `TagBuilder` cannot be empty and `HtmlString` overrides `ToString()`
 - `TagBuilder.ToString()` (now the type's `FullName`) is also never empty
@Eilon Eilon added the bug label Sep 26, 2016
dougbu added a commit that referenced this issue Sep 27, 2016
- #5317
- worked only because `TagBuilder` cannot be empty and `HtmlString` overrides `ToString()`
 - `TagBuilder.ToString()` (now the type's `FullName`) is also never empty
dougbu added a commit that referenced this issue Sep 27, 2016
- #5317
- previously worked only because `TagBuilder` cannot be empty and `HtmlString` overrides `ToString()`
 - `TagBuilder.ToString()` (now the type's `FullName`) is also never empty
- copy `NullHtmlEncoder` from Razor and give it a better name (`PassThroughHtmlEncoder`)
 - not available in this project and (from its namespace) not intended for general use
@dougbu
Copy link
Member Author

dougbu commented Sep 27, 2016

f222fa4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants