Skip to content

Conversation

@krichprollsch
Copy link
Member

@krichprollsch krichprollsch commented Jul 31, 2025

Our toInterface expect the given Element is always an HTMLElement.
So I think it's better to create always HTMLElement. No b/c we could want to create a non-HTML tag, eg. svg

But I would also like to detect if an Element is really an HTMLElement to avoid crash on toInterface, but I can't figure out one correct solution... done using _dom_element_get_tag_name ✔️

depends on lightpanda-io/libdom#32

@karlseguin
Copy link
Collaborator

karlseguin commented Aug 1, 2025

I think you could stop it from crashing by calling _dom_element_get_tag_name (which I don't think we have exposed in netsurf.zig) instead of _dom_html_element_get_tag_type ?? You could then maybe handle non-HTML tag names (svg, ...??).

@krichprollsch
Copy link
Member Author

So as you suggested, as used _dom_element_get_tag_name + Tag.fromString instead of _dom_html_element_get_tag_type to retrieve the element's Tag.
I think it makes more sense this way: Node.toInterface => Element.toInterface => HTMLElement.toInterfaceFromTag

@krichprollsch
Copy link
Member Author

I pushed more changed to remove netsurf.elementHTMLGetTagType

But i have html/dom/documents/dom-tree-accessors/document.title-09.html crashing again (on c._dom_html_document_create_element_ns).

so still in progress...

@krichprollsch krichprollsch force-pushed the html_element-and-element branch 3 times, most recently from e75ba67 to ce83e68 Compare August 4, 2025 12:29
@krichprollsch
Copy link
Member Author

@karlseguin WPT tests are passing now.

@krichprollsch
Copy link
Member Author

krichprollsch commented Aug 4, 2025

Hum, Maybe I should keep try Element.toInterface signature unchanged and create a toInterfaceT(comptime T: type, e: *parser.Element) !T { instead to avoid all these changes 🤔

@krichprollsch krichprollsch force-pushed the html_element-and-element branch from ce83e68 to 39fc5a7 Compare August 4, 2025 13:11
@krichprollsch
Copy link
Member Author

ok here is a cleaner version

@krichprollsch krichprollsch self-assigned this Aug 4, 2025
return doc_html;
}

pub inline fn documentCreateHTMLElement(doc: *Document, tag_name: []const u8) !*Element {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know everything in netsurf.zig is inlined, but Andrew's not a fan:

https://ziggit.dev/t/inlining-functions/1341/3

return doc_html;
}

pub inline fn documentCreateHTMLElement(doc: *Document, tag_name: []const u8) !*Element {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to keep the originals? If our document is always an html document, then it will always create an HTMLElement. Only reason I can see to keep both is if we think we'll have a non-html document??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can create non-html document from scratch in JS and then add elements to them.
If I try to use only HTML element creation, WPT test is crashing on https://github.com/lightpanda-io/wpt/blob/main/html/dom/documents/dom-tree-accessors/document.title-09.html.

It's b/c the document isn't an HTML one.

So depending the document, we want to create HTMLElement or Element...
I could maybe detect the document's type and use the best function...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid I can't find a way to detect if a document is an HTML one...
The constructor doesn't set any base doc attribute with a specific value.
https://github.com/lightpanda-io/libdom/blob/master/src/html/html_document.c#L93-L116

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I did with lightpanda-io/libdom#32

@karlseguin
Copy link
Collaborator

LGTM

@krichprollsch krichprollsch force-pushed the html_element-and-element branch from 3ec792c to 0fee2bb Compare August 6, 2025 08:43
@krichprollsch krichprollsch merged commit 84d07f3 into main Aug 6, 2025
10 checks passed
@krichprollsch krichprollsch deleted the html_element-and-element branch August 6, 2025 08:55
@github-actions github-actions bot locked and limited conversation to collaborators Aug 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants