Skip to content

Commit

Permalink
fix: element comparer includes toggle to enforce check of closing tag
Browse files Browse the repository at this point in the history
  • Loading branch information
egil committed Mar 5, 2023
1 parent 6fb2ebe commit 38875ff
Show file tree
Hide file tree
Showing 8 changed files with 415 additions and 455 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 0.18.1

- Fixed element comparer such that it can strictly check if the closing tags in the source markup is the same.

# 0.18.0

- Added a new comparer, which ensures element tags are closed the same way, e.g. `<br> and <br />` would not be considered equal, but `<br>` and `<br>` would be.
Expand Down
609 changes: 301 additions & 308 deletions src/AngleSharp.Diffing.Tests/Strategies/DiffingStrategyPipelineTest.cs

Large diffs are not rendered by default.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
namespace AngleSharp.Diffing.Strategies.ElementStrategies;

public class ElementComparerTest : DiffingTestBase
{
public ElementComparerTest(DiffingTestFixture fixture) : base(fixture)
{
}

[Theory(DisplayName = "When control and test nodes have the same type and name and enforceTagClosing is false, the result is Same")]
[InlineData("<p>", "<p />")]
[InlineData("<br>", "<br/>")]
[InlineData("<br>", "<br>")]
[InlineData("<p>", "<p>")]
public void Test001(string controlHtml, string testHtml)
{
var comparison = ToComparison(controlHtml, testHtml);
new ElementComparer(enforceTagClosing: false)
.Compare(comparison, CompareResult.Unknown)
.ShouldBe(CompareResult.Same);
}

[Theory(DisplayName = "When control and test nodes have the a different type and name, the result is Different")]
[InlineData("<div>", "<p>", false)]
[InlineData("<div>", "textnode", false)]
[InlineData("<div>", "<!--comment-->", false)]
[InlineData("<!--comment-->", "textnode", false)]
[InlineData("<br>", "<br/>", true)]
[InlineData("<input>", "<input/>", true)]
[InlineData("<div>", "<p>", true)]
[InlineData("<div>", "textnode", true)]
[InlineData("<div>", "<!--comment-->", true)]
[InlineData("<!--comment-->", "textnode", true)]
public void Test002(string controlHtml, string testHtml, bool enforceTagClosing)
{
var comparison = ToComparison(controlHtml, testHtml);
new ElementComparer(enforceTagClosing)
.Compare(comparison, CompareResult.Unknown)
.ShouldBe(CompareResult.Different);
}

[Theory(DisplayName = "When unknown node is used in comparison, but node name is equal, the result is Same")]
[InlineData("<svg><path></path></svg>", "<path/>")]
public void HandleUnknownNodeDuringComparison(string controlHtml, string testHtml)
{
var knownNode = ToNode(controlHtml).FirstChild.ToComparisonSource(0, ComparisonSourceType.Control);
var unknownNode = ToNode(testHtml).ToComparisonSource(0, ComparisonSourceType.Test);
var comparison = new Comparison(knownNode, unknownNode);

new ElementComparer(enforceTagClosing: false)
.Compare(comparison, CompareResult.Unknown)
.ShouldBe(CompareResult.Same);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,19 @@ namespace AngleSharp.Diffing;

public static partial class DiffingStrategyPipelineBuilderExtensions
{
/// <summary>
/// Enables the basic element comparer, that checks if two nodes are element nodes and have the same name.
/// </summary>
/// <param name="builder">The collection to add the comparer to.</param>
/// <param name="enforceTagClosing">
/// Whether or not the closing style of
/// an element is considered during comparison.
/// </param>
public static IDiffingStrategyCollection AddElementComparer(this IDiffingStrategyCollection builder, bool enforceTagClosing)
{
/// <summary>
/// Enables the basic element comparer, that checks if two nodes are element nodes and have the same name.
/// </summary>
public static IDiffingStrategyCollection AddElementComparer(this IDiffingStrategyCollection builder)
{
builder.AddComparer(ElementComparer.Compare, StrategyType.Generalized);
return builder;
}

/// <summary>
/// Adds an element comparer that ensures element tags are closed the same way, e.g. `&lt;br&gt;` and `&lt;br /&gt;` would not be considered equal, but `&lt;br&gt;` and `&lt;br&gt;` would be.
/// </summary>
public static IDiffingStrategyCollection AddElementClosingComparer(this IDiffingStrategyCollection builder)
{
builder.AddComparer(ElementClosingComparer.Compare, StrategyType.Generalized);
return builder;
}
builder.AddComparer(new ElementComparer(enforceTagClosing).Compare, StrategyType.Generalized);
return builder;
}

/// <summary>
/// Enables the CSS-selector matcher strategy during diffing.
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,23 +1,55 @@
using AngleSharp.Diffing.Core;
using AngleSharp.Dom;
using AngleSharp.Html.Parser.Tokens;

namespace AngleSharp.Diffing.Strategies.ElementStrategies
namespace AngleSharp.Diffing.Strategies.ElementStrategies;

/// <summary>
/// Represents the element comparer strategy.
/// </summary>
public class ElementComparer
{
/// <summary>
/// Represents the element comparer strategy.
/// Gets whether or not the closing style of
/// an element is considered during comparison.
/// </summary>
public bool EnforceTagClosing { get; }

/// <summary>
/// Creates an instance of the <see cref="ElementComparer"/>.
/// </summary>
/// <param name="enforceTagClosing">
/// Whether or not the closing style of
/// an element is considered during comparison.
/// </param>
public ElementComparer(bool enforceTagClosing)
{
EnforceTagClosing = enforceTagClosing;
}

/// <summary>
/// The element comparer strategy.
/// </summary>
public static class ElementComparer
public CompareResult Compare(in Comparison comparison, CompareResult currentDecision)
{
/// <summary>
/// The element comparer strategy.
/// </summary>
public static CompareResult Compare(in Comparison comparison, CompareResult currentDecision)
if (currentDecision.IsSameOrSkip())
return currentDecision;

var result = comparison.Control.Node.NodeType == NodeType.Element && comparison.AreNodeTypesEqual
? CompareResult.Same
: CompareResult.Different;

if (EnforceTagClosing && result == CompareResult.Same)
{
if (currentDecision.IsSameOrSkip())
return currentDecision;
return comparison.Control.Node.NodeType == NodeType.Element && comparison.AreNodeTypesEqual
? CompareResult.Same
if (comparison.Test.Node is not IElement testElement || testElement.SourceReference is not HtmlTagToken testTag)
throw new InvalidOperationException("No source reference attached to test element, cannot determine element tag closing style.");

if (comparison.Control.Node is not IElement controlElement || controlElement.SourceReference is not HtmlTagToken controlTag)
throw new InvalidOperationException("No source reference attached to test element, cannot determine element tag closing style.");

return testTag.IsSelfClosing == controlTag.IsSelfClosing
? result
: CompareResult.Different;
}

return result;
}
}

0 comments on commit 38875ff

Please sign in to comment.