Implement JavaScript Text constructor#249
Conversation
WalkthroughThe changes implement Text constructor support in the DOM scripting layer. This includes modifying constructor validation logic to use dynamic checks instead of hard-coded exceptions, adding CreateText and CreateTextData utility functions, creating a TextConstructor generic function, and registering the Text constructor in the window realm configuration. Supporting tests and test utilities are also added, including a new testText function to verify Text node construction functionality. 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| for cons := range d.IdlInterface.InternalSpec.Constructors() { | ||
| // I can't find documenation; but constructors with [HTMLContructor] | ||
| // extended attribute are not constructable; i.e., they throw an | ||
| // "invalid constructor" TypeError | ||
| for _, attr := range cons.ExtAttrs { | ||
| if attr.Name == "HTMLConstructor" { | ||
| return false | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for webref/idl Interface struct definition and HTMLConstructor representation
rg -n "type Interface struct" -g '*.go' -A 20Repository: gost-dom/browser
Length of output: 42
🏁 Script executed:
# Search for HTMLConstructor usage in the codebase
rg -n "HTMLConstructor" -g '*.*'Repository: gost-dom/browser
Length of output: 158
🏁 Script executed:
# Search for how constructors are handled and checked
rg -n "AllowConstructor" -g '*.go' -B 5 -A 10Repository: gost-dom/browser
Length of output: 4229
🏁 Script executed:
# Look at webref package structure
find . -name "*.go" -path "*/webref*" -type f | head -20Repository: gost-dom/browser
Length of output: 42
🏁 Script executed:
# Search for ExtAttrs patterns related to Interface or Constructors
rg -n "ExtAttrs" -g '*.go' | head -30Repository: gost-dom/browser
Length of output: 158
🏁 Script executed:
# Find webref package imports and dependencies
rg -n "webref" -g '*.go' | head -20Repository: gost-dom/browser
Length of output: 1856
🏁 Script executed:
# Check go.mod for webref dependency
cat go.mod | grep -i webrefRepository: gost-dom/browser
Length of output: 42
🏁 Script executed:
# Search for InternalSpec definition
rg -n "InternalSpec" -g '*.go' -A 5 | head -40Repository: gost-dom/browser
Length of output: 1160
🏁 Script executed:
# Check the IdlInterface type definition
rg -n "type IdlInterface" -g '*.go' -A 10Repository: gost-dom/browser
Length of output: 3961
🏁 Script executed:
# Search for where Interface and its ExtAttrs are used
rg -n "\.ExtAttrs" -g '*.go'Repository: gost-dom/browser
Length of output: 158
🏁 Script executed:
# Look for InternalSpec field definition in IdlInterface or related files
rg -n "InternalSpec" -g '*.go' -B 3 -A 3Repository: gost-dom/browser
Length of output: 1263
🏁 Script executed:
# Check the full IdlInterface struct definition including any embedded fields
cat -n internal/code-gen/interfaces/idl_interface.go | sed -n '18,35p'Repository: gost-dom/browser
Length of output: 711
🏁 Script executed:
# Search for where IdlInterface is embedded or created to understand its relationship with webref
rg -n "IdlInterface.*{" -g '*.go' -A 10 | head -50Repository: gost-dom/browser
Length of output: 153
🏁 Script executed:
# Look for webref idl.Interface usage to understand the data model
rg -n "idl\.Interface" -g '*.go' | head -20Repository: gost-dom/browser
Length of output: 1934
🌐 Web query:
github.com/gost-dom/webref idl.Interface HTMLConstructor extended attribute
💡 Result:
[HTMLConstructor] is not a Web IDL standard extended attribute; it’s defined by the HTML Standard and is only valid on interfaces (no args; must not be combined with [Constructor]/[NoInterfaceObject], etc.) [1]. The Web IDL Standard’s own list of interface-level extended attributes does not include HTMLConstructor [4].
In github.com/gost-dom/webref v0.5.7, the high-level idl.Interface type doesn’t expose interface extended attributes like HTMLConstructor (it contains things like inheritance, members, iterable types, constructors, etc.) [2]. Those “raw” extended attributes are instead available via the legacy JSON model, which includes extAttrs as []legacy.ExtAttr on types like legacy.ExtendedName [3].
🌐 Web query:
webref idl Interface ExtAttrs extended attributes structure
💡 Result:
Web IDL: where interface ExtAttrs (extended attributes) live
In the Web IDL grammar, extended attributes appear as an ExtendedAttributeList: a square‑bracketed, comma‑separated list placed before the thing being annotated (including an interface). The spec defines it as an annotation mechanism and gives the core productions, e.g. ExtendedAttributeList → [ … ] or empty. (webidl.spec.whatwg.org)
Example (interface-level ext attrs):
[Exposed=Window, SecureContext]
interface Foo { };“Structure” in practice (WebIDL2 / WebRef parser AST)
Tools like WebIDL2 (commonly used by WebRef pipelines) represent those interface extended attributes on the parsed node as an extAttrs array. Each item is an object like: (chromium.googlesource.com)
{
"name": "Exposed",
"arguments": null,
"type": "extended-attribute",
"rhs": { "type": "identifier-list", "value": ["Window", "Worker"] }
}Field meanings (per WebIDL2 docs): (chromium.googlesource.com)
name: extended attribute nametype:"extended-attribute"rhs: present when there’s a right-hand side (e.g.=Windowor=(Window,Worker)), withtypelike"identifier"/"identifier-list"arguments: argument list if present (e.g.[Foo(x)]), otherwisenull(empty list is[], absent isnull)
Citations:
- 1: https://webidl.spec.whatwg.org/?utm_source=openai
- 2: https://chromium.googlesource.com/external/w3c/web-platform-tests/%2B/merge_pr_11582/resources/webidl2/README.md?utm_source=openai
- 3: https://chromium.googlesource.com/external/w3c/web-platform-tests/%2B/merge_pr_11582/resources/webidl2/README.md?utm_source=openai
🌐 Web query:
"HTMLConstructor" Web IDL interface vs constructor extended attribute
💡 Result:
There isn’t a Web IDL interface named HTMLConstructor in the platform specs; [HTMLConstructor] is an IDL extended attribute defined by the HTML Standard for HTML element interfaces.
[Constructor] (Web IDL)
- A generic Web IDL mechanism: putting
[Constructor]on an interface makes the interface object “constructable” (has[[Construct]]), with the argument list you declare. (w3.org)
[HTMLConstructor] (HTML Standard)
- A special-case hook for HTML elements (needed for custom elements): it marks that the interface object’s call/construct behavior follows the HTML element constructor algorithm (not a normal Web IDL constructor). (wicg.github.io)
- Constraints: it takes no arguments, appears only on interfaces, only once, and must not be combined with
[Constructor](or[NoInterfaceObject]). (wicg.github.io) - Engine docs summarize the intent similarly: “HTML Elements have special constructor behavior”; cannot be annotated with
[Constructor]. (chromium.googlesource.com)
Practical takeaway
- Use
[Constructor]for “normal” constructable Web IDL types. - Use
[HTMLConstructor]only for HTML element interfaces, where construction must integrate with the HTML/custom-elements construction rules (and therefore is intentionally not expressed as a normal Web IDL[Constructor]). (wicg.github.io)
Citations:
- 1: https://www.w3.org/2017/08/webidl-fpwd-draft.html?utm_source=openai
- 2: https://wicg.github.io/controls-list/html-output/?utm_source=openai
- 3: https://wicg.github.io/controls-list/html-output/?utm_source=openai
- 4: https://chromium.googlesource.com/chromium/src/%2Bshow/HEAD/third_party/blink/renderer/bindings/IDLExtendedAttributes.md?utm_source=openai
- 5: https://wicg.github.io/controls-list/html-output/?utm_source=openai
The [HTMLConstructor] check is incomplete: it appears at the interface level, not constructor level.
Per the HTML Standard, [HTMLConstructor] is an interface-level extended attribute only (never on constructors). The current code checks constructor-level ExtAttrs, which won't catch interface-level [HTMLConstructor] declarations. Additionally, webref's public idl.Interface API does not expose interface extended attributes, making it impossible to verify this constraint with the current data model. An interface-level check is needed, or the data model must be extended to provide access to interface ExtAttrs.
| func testText(t *testing.T, e html.ScriptEngine) { | ||
| w := browsertest.InitWindow(t, e) | ||
| res := w.MustEval("new Text('Text content')") | ||
| _, ok := res.(dom.Text) | ||
| if !assert.True(t, ok) { | ||
| t.Logf("Not text: %T", res) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add an assertion for constructor argument wiring.
Right now the test only checks the type; asserting the data would validate that the constructor uses the provided string.
Proposed test enhancement
- res := w.MustEval("new Text('Text content')")
- _, ok := res.(dom.Text)
- if !assert.True(t, ok) {
- t.Logf("Not text: %T", res)
- }
+ res := w.MustEval("new Text('Text content')")
+ text, ok := res.(dom.Text)
+ if assert.True(t, ok) {
+ assert.Equal(t, "Text content", text.Data())
+ } else {
+ t.Logf("Not text: %T", res)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func testText(t *testing.T, e html.ScriptEngine) { | |
| w := browsertest.InitWindow(t, e) | |
| res := w.MustEval("new Text('Text content')") | |
| _, ok := res.(dom.Text) | |
| if !assert.True(t, ok) { | |
| t.Logf("Not text: %T", res) | |
| } | |
| func testText(t *testing.T, e html.ScriptEngine) { | |
| w := browsertest.InitWindow(t, e) | |
| res := w.MustEval("new Text('Text content')") | |
| text, ok := res.(dom.Text) | |
| if assert.True(t, ok) { | |
| assert.Equal(t, "Text content", text.Data()) | |
| } else { | |
| t.Logf("Not text: %T", res) | |
| } |
1 participant
No description provided.