From 19df73729ab9b16510c8eac2d9c6c97a294571ce Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 4 Jun 2025 18:04:39 +0800 Subject: [PATCH] Improve usability of NodeWrapper The NodeWrapper pattern attaches a Zig instance to a libdom Node. That works in isolation, but for 1 given node, we might want to attach different instances. For example, for an HTMLScriptElement we want to attach an `onError`, but for that same node viewed as an HTMLElement we want to a `CSSStyleDeclaration`. We can only have one. Currently, this code will crash if, for example, we create the embedded data as an HTMLScriptElement, then try to read the embedded data as an HTMLElement. This PR introduces dedicated state class. So if you want the onError property, you no longer ask the NodeWrapper for an HTMLSCriptElement. Instead, you ask for a storage/HTMLElement. Nothing fancy here, just memory-inefficient optional fields. If it gets out of hand, we'll think of something more clever. --- src/browser/dom/document.zig | 20 +++++++++----- src/browser/html/document.zig | 29 ++++++++------------ src/browser/html/elements.zig | 45 ++++++++++++------------------- src/browser/page.zig | 4 +-- src/browser/state/Document.zig | 31 +++++++++++++++++++++ src/browser/state/HTMLElement.zig | 24 +++++++++++++++++ 6 files changed, 99 insertions(+), 54 deletions(-) create mode 100644 src/browser/state/Document.zig create mode 100644 src/browser/state/HTMLElement.zig diff --git a/src/browser/dom/document.zig b/src/browser/dom/document.zig index 4d6832e3..f96e20bb 100644 --- a/src/browser/dom/document.zig +++ b/src/browser/dom/document.zig @@ -28,6 +28,7 @@ const NodeUnion = @import("node.zig").Union; const collection = @import("html_collection.zig"); const css = @import("css.zig"); +const State = @import("../state/Document.zig"); const Element = @import("element.zig").Element; const ElementUnion = @import("element.zig").Union; const TreeWalker = @import("tree_walker.zig").TreeWalker; @@ -42,8 +43,6 @@ pub const Document = struct { pub const prototype = *Node; pub const subtype = .node; - active_element: ?*parser.Element = null, - pub fn constructor(page: *const Page) !*parser.DocumentHTML { const doc = try parser.documentCreateDocument( try parser.documentHTMLGetTitle(page.window.document), @@ -245,9 +244,9 @@ pub const Document = struct { return try TreeWalker.init(root, what_to_show, filter); } - pub fn get_activeElement(doc: *parser.Document, page: *Page) !?ElementUnion { - const self = try page.getOrCreateNodeWrapper(Document, @ptrCast(doc)); - if (self.active_element) |ae| { + pub fn get_activeElement(self: *parser.Document, page: *Page) !?ElementUnion { + const state = try page.getOrCreateNodeWrapper(State, @ptrCast(self)); + if (state.active_element) |ae| { return try Element.toInterface(ae); } @@ -255,7 +254,16 @@ pub const Document = struct { return try Element.toInterface(@ptrCast(body)); } - return get_documentElement(doc); + return get_documentElement(self); + } + + // TODO: some elements can't be focused, like if they're disabled + // but there doesn't seem to be a generic way to check this. For example + // we could look for the "disabled" attribute, but that's only meaningful + // on certain types, and libdom's vtable doesn't seem to expose this. + pub fn setFocus(self: *parser.Document, e: *parser.ElementHTML, page: *Page) !void { + const state = try page.getOrCreateNodeWrapper(State, @ptrCast(self)); + state.active_element = @ptrCast(e); } }; diff --git a/src/browser/html/document.zig b/src/browser/html/document.zig index 5c74fe7e..384bc9d7 100644 --- a/src/browser/html/document.zig +++ b/src/browser/html/document.zig @@ -30,6 +30,7 @@ const NodeList = @import("../dom/nodelist.zig").NodeList; const Location = @import("location.zig").Location; const collection = @import("../dom/html_collection.zig"); +const State = @import("../state/Document.zig"); const Walker = @import("../dom/walker.zig").WalkerDepthFirst; const Cookie = @import("../storage/cookie.zig").Cookie; @@ -39,14 +40,6 @@ pub const HTMLDocument = struct { pub const prototype = *Document; pub const subtype = .node; - ready_state: ReadyState = .loading, - - const ReadyState = enum { - loading, - interactive, - complete, - }; - // JS funcs // -------- @@ -191,9 +184,9 @@ pub const HTMLDocument = struct { return &page.window; } - pub fn get_readyState(node: *parser.DocumentHTML, page: *Page) ![]const u8 { - const self = try page.getOrCreateNodeWrapper(HTMLDocument, @ptrCast(node)); - return @tagName(self.ready_state); + pub fn get_readyState(self: *parser.DocumentHTML, page: *Page) ![]const u8 { + const state = try page.getOrCreateNodeWrapper(State, @ptrCast(self)); + return @tagName(state.ready_state); } // noop legacy functions @@ -270,9 +263,9 @@ pub const HTMLDocument = struct { return list.items; } - pub fn documentIsLoaded(html_doc: *parser.DocumentHTML, page: *Page) !void { - const self = try page.getOrCreateNodeWrapper(HTMLDocument, @ptrCast(html_doc)); - self.ready_state = .interactive; + pub fn documentIsLoaded(self: *parser.DocumentHTML, page: *Page) !void { + const state = try page.getOrCreateNodeWrapper(State, @ptrCast(self)); + state.ready_state = .interactive; const evt = try parser.eventCreate(); defer parser.eventDestroy(evt); @@ -282,12 +275,12 @@ pub const HTMLDocument = struct { .source = "document", }); try parser.eventInit(evt, "DOMContentLoaded", .{ .bubbles = true, .cancelable = true }); - _ = try parser.eventTargetDispatchEvent(parser.toEventTarget(parser.DocumentHTML, html_doc), evt); + _ = try parser.eventTargetDispatchEvent(parser.toEventTarget(parser.DocumentHTML, self), evt); } - pub fn documentIsComplete(html_doc: *parser.DocumentHTML, page: *Page) !void { - const self = try page.getOrCreateNodeWrapper(HTMLDocument, @ptrCast(html_doc)); - self.ready_state = .complete; + pub fn documentIsComplete(self: *parser.DocumentHTML, page: *Page) !void { + const state = try page.getOrCreateNodeWrapper(State, @ptrCast(self)); + state.ready_state = .complete; } }; diff --git a/src/browser/html/elements.zig b/src/browser/html/elements.zig index d0bb23b4..5dae5c2a 100644 --- a/src/browser/html/elements.zig +++ b/src/browser/html/elements.zig @@ -25,6 +25,7 @@ const Page = @import("../page.zig").Page; const urlStitch = @import("../../url.zig").URL.stitch; const URL = @import("../url/url.zig").URL; const Node = @import("../dom/node.zig").Node; +const State = @import("../state/HTMLElement.zig"); const Element = @import("../dom/element.zig").Element; const CSSStyleDeclaration = @import("../cssom/css_style_declaration.zig").CSSStyleDeclaration; @@ -112,11 +113,9 @@ pub const HTMLElement = struct { pub const prototype = *Element; pub const subtype = .node; - style: CSSStyleDeclaration = .empty, - pub fn get_style(e: *parser.ElementHTML, page: *Page) !*CSSStyleDeclaration { - const self = try page.getOrCreateNodeWrapper(HTMLElement, @ptrCast(e)); - return &self.style; + const state = try page.getOrCreateNodeWrapper(State, @ptrCast(e)); + return &state.style; } pub fn get_innerText(e: *parser.ElementHTML) ![]const u8 { @@ -159,16 +158,9 @@ pub const HTMLElement = struct { return; } - const root_node = try parser.nodeGetRootNode(@ptrCast(e)); - const Document = @import("../dom/document.zig").Document; - const document = try page.getOrCreateNodeWrapper(Document, @ptrCast(root_node)); - - // TODO: some elements can't be focused, like if they're disabled - // but there doesn't seem to be a generic way to check this. For example - // we could look for the "disabled" attribute, but that's only meaningful - // on certain types, and libdom's vtable doesn't seem to expose this. - document.active_element = @ptrCast(e); + const root_node = try parser.nodeGetRootNode(@ptrCast(e)); + try Document.setFocus(@ptrCast(root_node), e, page); } }; @@ -852,9 +844,6 @@ pub const HTMLScriptElement = struct { pub const prototype = *HTMLElement; pub const subtype = .node; - onload: ?Env.Function = null, - onerror: ?Env.Function = null, - pub fn get_src(self: *parser.Script) !?[]const u8 { return try parser.elementGetAttribute( parser.scriptToElt(self), @@ -964,24 +953,24 @@ pub const HTMLScriptElement = struct { return try parser.elementRemoveAttribute(parser.scriptToElt(self), "nomodule"); } - pub fn get_onload(script: *parser.Script, page: *Page) !?Env.Function { - const self = page.getNodeWrapper(HTMLScriptElement, @ptrCast(script)) orelse return null; - return self.onload; + pub fn get_onload(self: *parser.Script, page: *Page) !?Env.Function { + const state = page.getNodeWrapper(State, @ptrCast(self)) orelse return null; + return state.onload; } - pub fn set_onload(script: *parser.Script, function: ?Env.Function, page: *Page) !void { - const self = try page.getOrCreateNodeWrapper(HTMLScriptElement, @ptrCast(script)); - self.onload = function; + pub fn set_onload(self: *parser.Script, function: ?Env.Function, page: *Page) !void { + const state = try page.getOrCreateNodeWrapper(State, @ptrCast(self)); + state.onload = function; } - pub fn get_onerror(script: *parser.Script, page: *Page) !?Env.Function { - const self = page.getNodeWrapper(HTMLScriptElement, @ptrCast(script)) orelse return null; - return self.onerror; + pub fn get_onerror(self: *parser.Script, page: *Page) !?Env.Function { + const state = page.getNodeWrapper(State, @ptrCast(self)) orelse return null; + return state.onerror; } - pub fn set_onerror(script: *parser.Script, function: ?Env.Function, page: *Page) !void { - const self = try page.getOrCreateNodeWrapper(HTMLScriptElement, @ptrCast(script)); - self.onerror = function; + pub fn set_onerror(self: *parser.Script, function: ?Env.Function, page: *Page) !void { + const state = try page.getOrCreateNodeWrapper(State, @ptrCast(self)); + state.onerror = function; } }; diff --git a/src/browser/page.zig b/src/browser/page.zig index cd633a3d..fa8088a9 100644 --- a/src/browser/page.zig +++ b/src/browser/page.zig @@ -743,8 +743,8 @@ const Script = struct { // attached to it. But this seems quite unlikely and it does help // optimize loading scripts, of which there can be hundreds for a // page. - const HTMLScriptElement = @import("html/elements.zig").HTMLScriptElement; - if (page.getNodeWrapper(HTMLScriptElement, @ptrCast(e))) |se| { + const State = @import("state/HTMLElement.zig"); + if (page.getNodeWrapper(State, @ptrCast(e))) |se| { if (se.onload) |function| { onload = .{ .function = function }; } diff --git a/src/browser/state/Document.zig b/src/browser/state/Document.zig new file mode 100644 index 00000000..dc2b6050 --- /dev/null +++ b/src/browser/state/Document.zig @@ -0,0 +1,31 @@ +// Copyright (C) 2023-2024 Lightpanda (Selecy SAS) +// +// Francis Bouvier +// Pierre Tachoire +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +const parser = @import("../netsurf.zig"); + +// proxy-owner for html/document +ready_state: ReadyState = .loading, + +// proxy-owner for dom/document +active_element: ?*parser.Element = null, + +const ReadyState = enum { + loading, + interactive, + complete, +}; diff --git a/src/browser/state/HTMLElement.zig b/src/browser/state/HTMLElement.zig new file mode 100644 index 00000000..0a41b207 --- /dev/null +++ b/src/browser/state/HTMLElement.zig @@ -0,0 +1,24 @@ +// Copyright (C) 2023-2024 Lightpanda (Selecy SAS) +// +// Francis Bouvier +// Pierre Tachoire +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +const Env = @import("../env.zig").Env; +const CSSStyleDeclaration = @import("../cssom/css_style_declaration.zig").CSSStyleDeclaration; + +onload: ?Env.Function = null, +onerror: ?Env.Function = null, +style: CSSStyleDeclaration = .empty,