Skip to content

INode.HasChildNodes is a property, and according to the spec it should be a method #1219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
5 tasks done
arekdygas opened this issue Mar 24, 2025 · 5 comments · Fixed by #1224 · May be fixed by AngleSharp/AngleSharp.Js#106
Closed
5 tasks done

INode.HasChildNodes is a property, and according to the spec it should be a method #1219

arekdygas opened this issue Mar 24, 2025 · 5 comments · Fixed by #1224 · May be fixed by AngleSharp/AngleSharp.Js#106
Labels
bug dom
Milestone
1.3.0

Comments

@arekdygas
Copy link
Contributor

Prerequisites

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

Description

Being a getter only property is not a problem in AngleSharp itself, but when it is exposed to JS DOM, via DomNameAttribute, then it doesn't conform to the spec.

I decided to report here and not in AngleSharp.JS, as the code in current repo requires fix.

Tested on master versions of AngleSharp, AngleSharp.Js and Jint

Steps to Reproduce

var config = Configuration.Default.WithJs();
var context = BrowsingContext.New(config);
var document = await context.OpenAsync(req => req.Content("")).ConfigureAwait(false);
Console.WriteLine(document.ExecuteScript("document.createElement('div').hasChildNodes()"));

Expected Behavior

Console should output: False

Actual Behavior

Exception is thrown:

Jint.Runtime.JavaScriptException: 'Property 'hasChildNodes' of object is not a function'

Possible Solution / Known Workarounds

Workaround: using document.createElement('div').hasChildNodes (without parentheses)

Proposed solutions:

  1. Change HasChildNodes to method.
    Pros: JS signature is the same as the one in C#
    Cons: breaking change

  2. Introduce ExposeAsMethod property in DomNameAttribute and set it to true for that property.
    Pros: no breaking change in C#; there's still breaking change in JS if anyone used mentioned workaround, but impact would be much lower
    Cons: different C#/JS signatures
@arekdygas arekdygas added the bug label Mar 24, 2025
@FlorianRappl
Copy link
Contributor

That's right and it would definitely be solved via (2) - I think this should remain a property in .NET.

I think the potentially best way forward would be an additional DomAccessor attribute - extending the Accessors with a Method constant (https://github.com/AngleSharp/AngleSharp/blob/devel/src/AngleSharp/Attributes/Accessors.cs). This way we can declare that the property should indeed be treated like a method.

@FlorianRappl FlorianRappl added this to the 1.3.0 milestone Mar 24, 2025
@arekdygas
Copy link
Contributor Author

arekdygas commented Mar 25, 2025 •
edited
Loading

I checked the code and see, that Accessors enum is used only for handling members in extensions, i.e. classes/interfaces/structs marked with DomExposedAttribute. This complicates things a bit. Normal properties, like HasChildNodes are handled separately, in SetNormalProperties method.

It would be possible to check for Accessors.Method within that method, and invoke SetMethod instead of SetProperty there if appropriate, but there's one question. Inside SetNormalProperties method, list of names is retrieved from DomName attributes, and these properties are set on the result object. Is it possible that we should register one of these names as property and one as a method? Seems unlikely, but if possible, then general DomAccessor would not fit there.

And besides - placing that code in SetNormalProperties seems a bit out of place...

@FlorianRappl
Copy link
Contributor

DomAccessor was meant for exactly those cases (C# definition not matching IDL exactly).

@arekdygas
Copy link
Contributor Author

I need a bit of help to fix this one.

Accessors are in AngleSharp, while handling them occurs in AngleSharp.Js, so two pull requests will be required. Even if the first one (AngleSharp) is accepted, AngleSharp.Js still references AS v. 1.1.2, so my changes won't compile. I know the problem might be trivial, but I have no idea how to handle such cases in GH projects.

Should I just create these pull request and while you will accept the first, the second one will wait until the reference is changed and then you might accept it?

@FlorianRappl
Copy link
Contributor

Yes, of course I would accept it - but you are right. Being able to fix it with just one PR would be better. I just don't see how. Whatever we do (new attribute, new convention, ...) [as long as we are not breaking] we will always need a PR in AngleSharp.Js.

I think this is fine, however. The PR in AngleSharp.Js is just to "teach" AngleSharp.Js about the new attribute / attribute usage.

This was referenced Apr 17, 2025
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug dom
Projects
None yet
Milestone
1.3.0
2 participants
@FlorianRappl @arekdygas