AngleSharp.Xml icon indicating copy to clipboard operation
AngleSharp.Xml copied to clipboard

Inserting child in self-closing element instead inserts element as sibling

Open heinrich-ulbricht opened this issue 1 year ago • 2 comments

Bug Report

Prerequisites

  • [x] Can you reproduce the problem in a MWE?
  • [x] Are you running the latest version of AngleSharp?
  • [ ] Did you check the FAQs to see if that helps you?
  • [x] Are you reporting to the correct repository? (there are multiple AngleSharp libraries, e.g., AngleSharp.Css for CSS support)
  • [x] Did you perform a search in the issues?

For more information, see the CONTRIBUTING guide.

Description

I have a self-closing element and now want to add a child element.

Steps to Reproduce

Here's a complete repo:

[TestMethod]
public void TestInsertChildToSelfClosingElementSimple()
{
    var xml =
    """
    <ac:structured-macro ac:name="children"/>
    """;

    var config = Configuration.Default.With(HtmlEntityProvider.Resolver).WithDefaultLoader(new LoaderOptions { IsResourceLoadingEnabled = true }).WithCss().WithXml();
    var context = BrowsingContext.New(config);
    var parser = new XmlParser(new XmlParserOptions(), context);

    var doc = parser.ParseDocument(xml);
    var parentElement = doc.DocumentElement;
    Assert.IsNotNull(parentElement);
    Assert.AreEqual(NodeFlags.SelfClosing, parentElement.Flags);

    var child = doc.CreateElement("ac:parameter");
    child.SetAttribute("ac:name", "dummy");
    parentElement.AppendChild(child);

    var parentElementXml = parentElement.ToXml();
    // it seems to be impossible to insert something into a self-closing element!? this test is here to document this fact
    Assert.AreEqual("""<ac:structured-macro ac:name="children" /><ac:parameter ac:name="dummy"></ac:parameter>""", parentElementXml);
}

Expected behavior: I expect the ac:parameter element to become a child element of the parent element. The outcome should look like this: <ac:structured-macro ac:name="children"><ac:parameter ac:name="dummy"></ac:parameter></ac:structured-macro>.

Actual behavior: The ac:parameter element is inserted as sibling instead: <ac:structured-macro ac:name="children" /><ac:parameter ac:name="dummy"></ac:parameter>.

Environment details: .NET 8, Windows 11 && Debian

Possible Solution

Cloning

I'd be fine with cloning the element if I could specify if the clone should be self-closing, or not. Basically optionally introduce the behavior that was once fixed here: #17 :D

I tried finding some specs that describe if it is valid to "transform" a self-closing element back to having children, but couldn't find anything. Cloning might introduce a compliant workaround if any specs prohibit changing the self-closing flag.

Changing the Flag

Maybe it is as easy as allowing to change the Flags property. Not sure about the consequences, though.

heinrich-ulbricht avatar Feb 18 '25 15:02 heinrich-ulbricht

Could be.

But I will most likely not have time to look into this in the near future. Contributions welcome.

FlorianRappl avatar Feb 18 '25 15:02 FlorianRappl

I resorted to this extension method to toggle the SelfClosing flag off:

public static void AsSelfClosing(this IElement element, bool selfClosing)
{
    const uint SelfClosing = 0x1;

    var type = typeof(IElement).Assembly.GetType("AngleSharp.Dom.Node");
    var field = type?.GetField("_flags", BindingFlags.Instance | BindingFlags.NonPublic);

    if (null == field)
    {
        return;
    }

    var flags = (uint)field.GetValue(element)!;
    if (selfClosing)
    {
        flags |= SelfClosing;
    } else 
    {
        flags &= ~SelfClosing;

    }
    field.SetValue(element, Enum.ToObject(field.FieldType, flags));
}

I found AsSelfClosing in a FAQ of yours and added the bool to choose whether it's on or off.

Toggling this flag is the key.

For a proper solution the following decisions need to be made:

  • Should the SelfClosing flag auto-clear implicitly on insert/append operations?
  • If not: Where is the right place to introduce an explicit clear operation? In Clone? As a separate method on IElement?

Currently, I don't have the knowledge to make good decisions.

heinrich-ulbricht avatar Feb 18 '25 21:02 heinrich-ulbricht