Change `[RestrictChildren]` to allow non-`TagHelper` tags.

- Updated the `TagHelperParseTreeRewriter` loosen child restrictions to non-`TagHelper` HTML elements. - Updated tests to showcase that non-`TagHelper` elements are allowed to be restricted. - Added an additional test case to showcase sub-sub nesting of non-`TagHelper` restricted children. #543
2015-09-24 12:28:46 -07:00 · 2015-09-24 12:28:46 -07:00 · d2a7a355cc
parent 816b1f4190
commit d2a7a355cc
6 changed files with 66 additions and 60 deletions
--- a/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/RestrictChildrenAttribute.cs
+++ b/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/RestrictChildrenAttribute.cs
@ -19,10 +19,10 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
        /// Instantiates a new instance of the <see cref="RestrictChildrenAttribute"/> class.
        /// </summary>
        /// <param name="childTag">
-        /// The tag name of an element allowed as a child. Tag helpers must target the element.
+        /// The tag name of an element allowed as a child.
        /// </param>
        /// <param name="childTags">
-        /// Additional names of elements allowed as children. Tag helpers must target all such elements.
+        /// Additional names of elements allowed as children.
        /// </param>
        public RestrictChildrenAttribute(string childTag, params string[] childTags)
        {
@ -35,7 +35,7 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
        }

        /// <summary>
-        /// Get the names of elements allowed as children. Tag helpers must target all such elements.
+        /// Get the names of elements allowed as children.
        /// </summary>
        public IEnumerable<string> ChildTags { get; }
    }
--- a/src/Microsoft.AspNet.Razor/Parser/TagHelpers/TagHelperParseTreeRewriter.cs
+++ b/src/Microsoft.AspNet.Razor/Parser/TagHelpers/TagHelperParseTreeRewriter.cs
@ -81,10 +81,10 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
                        }
                        else
                        {
-                            TrackTagBlock(childBlock);
-
                            // Non-TagHelper tag.
-                            ValidateParentTagHelperAllowsPlainTag(childBlock, context.ErrorSink);
+                            ValidateParentAllowsPlainTag(childBlock, context.ErrorSink);
+
+                            TrackTagBlock(childBlock);
                        }

                        // If we get to here it means that we're a normal html tag.  No need to iterate any deeper into
@ -99,7 +99,7 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
                }
                else
                {
-                    ValidateParentTagHelperAllowsContent((Span)child, context.ErrorSink);
+                    ValidateParentAllowsContent((Span)child, context.ErrorSink);
                }

                // At this point the child is a Span or Block with Type BlockType.Tag that doesn't happen to be a
@ -197,7 +197,7 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
                    return false;
                }

-                ValidateParentTagHelperAllowsTagHelper(tagName, tagBlock, context.ErrorSink);
+                ValidateParentAllowsTagHelper(tagName, tagBlock, context.ErrorSink);
                ValidateDescriptors(descriptors, tagName, tagBlock, context.ErrorSink);

                // We're in a start TagHelper block.
@ -277,7 +277,7 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
                    // can't recover it means there was no corresponding tag helper start tag.
                    if (TryRecoverTagHelper(tagName, tagBlock, context))
                    {
-                        ValidateParentTagHelperAllowsTagHelper(tagName, tagBlock, context.ErrorSink);
+                        ValidateParentAllowsTagHelper(tagName, tagBlock, context.ErrorSink);
                        ValidateTagSyntax(tagName, tagBlock, context);

                        // Successfully recovered, move onto the next element.
@ -335,10 +335,22 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
            return attributeNames;
        }

-        private void ValidateParentTagHelperAllowsContent(Span child, ErrorSink errorSink)
+        private bool HasAllowedChildren()
        {
-            var allowedChildren = _currentTagHelperTracker?.AllowedChildren;
-            if (allowedChildren != null)
+            var currentTracker = _trackerStack.Count > 0 ? _trackerStack.Peek() : null;
+
+            // If the current tracker is not a TagHelper then there's no AllowedChildren to enforce.
+            if (currentTracker == null || !currentTracker.IsTagHelper)
+            {
+                return false;
+            }
+
+            return _currentTagHelperTracker.AllowedChildren != null;
+        }
+
+        private void ValidateParentAllowsContent(Span child, ErrorSink errorSink)
+        {
+            if (HasAllowedChildren())
            {
                var content = child.Content;
                if (!string.IsNullOrWhiteSpace(content))
@ -347,6 +359,7 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
                    var whitespace = content.Substring(0, content.Length - trimmedStart.Length);
                    var errorStart = SourceLocation.Advance(child.Start, whitespace);
                    var length = trimmedStart.TrimEnd().Length;
+                    var allowedChildren = _currentTagHelperTracker.AllowedChildren;
                    var allowedChildrenString = string.Join(", ", allowedChildren);
                    errorSink.OnError(
                        errorStart,
@ -358,31 +371,32 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
            }
        }

-        private void ValidateParentTagHelperAllowsPlainTag(Block tagBlock, ErrorSink errorSink)
+        private void ValidateParentAllowsPlainTag(Block tagBlock, ErrorSink errorSink)
        {
-            if (_currentTagHelperTracker?.AllowedChildren != null)
+            var tagName = GetTagName(tagBlock);
+
+            // Treat partial tags such as '</' which have no tag names as content.
+            if (string.IsNullOrEmpty(tagName))
            {
-                var tagName = GetTagName(tagBlock);
+                Debug.Assert(tagBlock.Children.First() is Span);

-                // Treat partial tags such as '</' which have no tag names as content based errors.
-                if (string.IsNullOrEmpty(tagName))
-                {
-                    Debug.Assert(tagBlock.Children.First() is Span);
+                ValidateParentAllowsContent((Span)tagBlock.Children.First(), errorSink);
+                return;
+            }

-                    ValidateParentTagHelperAllowsContent(tagBlock.Children.First() as Span, errorSink);
-                    return;
-                }
+            var currentTracker = _trackerStack.Count > 0 ? _trackerStack.Peek() : null;

+            if (HasAllowedChildren() &&
+                !_currentTagHelperTracker.AllowedChildren.Contains(tagName, StringComparer.OrdinalIgnoreCase))
+            {
                OnAllowedChildrenTagError(_currentTagHelperTracker, tagName, tagBlock, errorSink);
            }
        }

-        private void ValidateParentTagHelperAllowsTagHelper(string tagName, Block tagBlock, ErrorSink errorSink)
+        private void ValidateParentAllowsTagHelper(string tagName, Block tagBlock, ErrorSink errorSink)
        {
-            var currentlyAllowedChildren = _currentTagHelperTracker?.PrefixedAllowedChildren;
-
-            if (currentlyAllowedChildren != null &&
-                !currentlyAllowedChildren.Contains(tagName, StringComparer.OrdinalIgnoreCase))
+            if (HasAllowedChildren() &&
+                !_currentTagHelperTracker.PrefixedAllowedChildren.Contains(tagName, StringComparer.OrdinalIgnoreCase))
            {
                OnAllowedChildrenTagError(_currentTagHelperTracker, tagName, tagBlock, errorSink);
            }
--- a/src/Microsoft.AspNet.Razor/Properties/RazorResources.Designer.cs
+++ b/src/Microsoft.AspNet.Razor/Properties/RazorResources.Designer.cs
@ -1531,7 +1531,7 @@ namespace Microsoft.AspNet.Razor
        }

        /// <summary>
-        /// The &lt;{0}&gt; tag is not allowed by parent &lt;{1}&gt; tag helper. Only child tag helper(s) targeting tag name(s) '{2}' are allowed.
+        /// The &lt;{0}&gt; tag is not allowed by parent &lt;{1}&gt; tag helper. Only child tags with name(s) '{2}' are allowed.
        /// </summary>
        internal static string TagHelperParseTreeRewriter_InvalidNestedTag
        {
@ -1539,7 +1539,7 @@ namespace Microsoft.AspNet.Razor
        }

        /// <summary>
-        /// The &lt;{0}&gt; tag is not allowed by parent &lt;{1}&gt; tag helper. Only child tag helper(s) targeting tag name(s) '{2}' are allowed.
+        /// The &lt;{0}&gt; tag is not allowed by parent &lt;{1}&gt; tag helper. Only child tags with name(s) '{2}' are allowed.
        /// </summary>
        internal static string FormatTagHelperParseTreeRewriter_InvalidNestedTag(object p0, object p1, object p2)
        {
--- a/src/Microsoft.AspNet.Razor/RazorResources.resx
+++ b/src/Microsoft.AspNet.Razor/RazorResources.resx
@ -423,7 +423,7 @@ Instead, wrap the contents of the block in "{{}}":
    <value>The parent &lt;{0}&gt; tag helper does not allow non-tag content. Only child tag helper(s) targeting tag name(s) '{1}' are allowed.</value>
  </data>
  <data name="TagHelperParseTreeRewriter_InvalidNestedTag" xml:space="preserve">
-    <value>The &lt;{0}&gt; tag is not allowed by parent &lt;{1}&gt; tag helper. Only child tag helper(s) targeting tag name(s) '{2}' are allowed.</value>
+    <value>The &lt;{0}&gt; tag is not allowed by parent &lt;{1}&gt; tag helper. Only child tags with name(s) '{2}' are allowed.</value>
  </data>
  <data name="ParseError_HelperDirectiveNotAvailable" xml:space="preserve">
    <value>The {0} directive is not supported.</value>
--- a/src/Microsoft.AspNet.Razor/TagHelpers/TagHelperDescriptor.cs
+++ b/src/Microsoft.AspNet.Razor/TagHelpers/TagHelperDescriptor.cs
@ -158,7 +158,7 @@ namespace Microsoft.AspNet.Razor.TagHelpers
        }

        /// <summary>
-        /// Get the names of elements allowed as children. Tag helpers must target all such elements.
+        /// Get the names of elements allowed as children.
        /// </summary>
        /// <remarks><c>null</c> indicates all children are allowed.</remarks>
        public IEnumerable<string> AllowedChildren { get; set; }
--- a/test/Microsoft.AspNet.Razor.Test/TagHelpers/TagHelperParseTreeRewriterTest.cs
+++ b/test/Microsoft.AspNet.Razor.Test/TagHelpers/TagHelperParseTreeRewriterTest.cs
@ -661,18 +661,6 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
                    lineIndex: 1,
                    columnIndex: 5,
                    length: 6),
-                new RazorError(
-                    RazorResources.FormatTagHelperParseTreeRewriter_CannotHaveNonTagContent("p", "br"),
-                    absoluteIndex: 23 + newLineLength * 2,
-                    lineIndex: 2,
-                    columnIndex: 8,
-                    length: 5),
-                new RazorError(
-                    RazorResources.FormatTagHelperParseTreeRewriter_InvalidNestedTag("strong", "p", "br"),
-                    absoluteIndex: 34 + newLineLength * 3,
-                    lineIndex: 3,
-                    columnIndex: 5,
-                    length: 6),
            };
            var expectedOutput = new MarkupBlock(
                new MarkupTagHelperBlock("p",
@ -711,13 +699,7 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
                    absoluteIndex: 18,
                    lineIndex: 0,
                    columnIndex: 18,
-                    length: 6),
-                new RazorError(
-                    RazorResources.FormatTagHelperParseTreeRewriter_InvalidNestedTag("strong", "strong", "br"),
-                    absoluteIndex: 27,
-                    lineIndex: 0,
-                    columnIndex: 27,
-                    length: 6),
+                    length: 6)
            };
            var expectedOutput = new MarkupBlock(
                new MarkupTagHelperBlock("strong",
@ -973,8 +955,6 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
                            nestedContentError("strong", "strong", 11, 6),
                            nestedTagError("br", "strong", "strong", 18, 2),
                            nestedTagError("em", "strong", "strong", 22, 2),
-                            nestedContentError("strong", "strong", 25, 11),
-                            nestedTagError("em", "strong", "strong", 38, 2),
                            nestedTagError("br", "p", "strong", 51, 2),
                            nestedContentError("p", "strong", 56, 9)
                        }
@ -995,13 +975,6 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
                                factory.Markup("Something"))),
                        new[]
                        {
-                            nestedTagError("custom", "p", "custom", 4, 6),
-                            nestedContentError("p", "custom", 11, 6),
-                            nestedTagError("br", "p", "custom", 18, 2),
-                            nestedTagError("em", "p", "custom", 22, 2),
-                            nestedContentError("p", "custom", 25, 11),
-                            nestedTagError("em", "p", "custom", 38, 2),
-                            nestedTagError("custom", "p", "custom", 43, 6),
                            nestedTagError("br", "p", "custom", 51, 2),
                            nestedContentError("p", "custom", 56, 9)
                        }
@ -1027,7 +1000,26 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
                        {
                            nestedContentError("p", "custom", 3, 1),
                        }
-                    }
+                    },
+                    {
+                        "<p><custom><br>:<strong><strong>Hello</strong></strong>:<input></custom></p>",
+                        new[] { "custom", "strong" },
+                        new MarkupBlock(
+                            new MarkupTagHelperBlock("p",
+                                blockFactory.MarkupTagBlock("<custom>"),
+                                new MarkupTagHelperBlock("br", TagMode.StartTagOnly),
+                                factory.Markup(":"),
+                                new MarkupTagHelperBlock("strong",
+                                    new MarkupTagHelperBlock("strong",
+                                        factory.Markup("Hello"))),
+                                factory.Markup(":"),
+                                blockFactory.MarkupTagBlock("<input>"),
+                                blockFactory.MarkupTagBlock("</custom>"))),
+                        new[]
+                        {
+                            nestedContentError("strong", "custom, strong", 32, 5),
+                        }
+                    },
                };
            }
        }