Skip to content

Commit 83a628a

Browse files
CopilotCopilot
authored
feat(dom): support Element.onclick attribute for inline event handlers (#309)
1 parent f1b92b1 commit 83a628a

13 files changed

Lines changed: 523 additions & 31 deletions

‎fixtures/html/element-onclick-test.html‎

Lines changed: 269 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,269 @@
1+
<!DOCTYPE html>
2+
<html>
3+
4+
<head>
5+
<meta charset="utf-8" />
6+
<title>OnClick Handler Test</title>
7+
<style>
8+
body {
9+
font-family: Arial, sans-serif;
10+
margin: 20px;
11+
padding: 20px;
12+
background-color: #f5f5f5;
13+
}
14+
15+
button {
16+
margin: 10px;
17+
padding: 10px;
18+
font-size: 16px;
19+
background-color: antiquewhite;
20+
border: 2px solid #ccc;
21+
border-radius: 5px;
22+
cursor: pointer;
23+
}
24+
25+
button:hover {
26+
background-color: #f0e68c;
27+
}
28+
29+
.test-section {
30+
background: white;
31+
border: 1px solid #ddd;
32+
border-radius: 8px;
33+
padding: 20px;
34+
margin: 20px 0;
35+
}
36+
37+
.results {
38+
background: #f9f9f9;
39+
border: 1px solid #ddd;
40+
border-radius: 5px;
41+
padding: 15px;
42+
margin-top: 20px;
43+
min-height: 200px;
44+
}
45+
46+
.log-entry {
47+
padding: 5px 0;
48+
border-bottom: 1px solid #eee;
49+
font-family: monospace;
50+
}
51+
52+
.log-entry.success {
53+
color: green;
54+
}
55+
56+
.log-entry.info {
57+
color: blue;
58+
}
59+
60+
.log-entry.error {
61+
color: red;
62+
}
63+
64+
.status {
65+
font-weight: bold;
66+
margin-top: 10px;
67+
}
68+
69+
.test-button-clicked {
70+
background-color: #90EE90 !important;
71+
animation: flash 0.5s;
72+
}
73+
74+
@keyframes flash {
75+
0% { background-color: #90EE90; }
76+
50% { background-color: #FFFF99; }
77+
100% { background-color: #90EE90; }
78+
}
79+
</style>
80+
</head>
81+
82+
<body>
83+
<h1>OnClick Handler Test</h1>
84+
85+
<div class="test-section">
86+
<h2>Interactive Tests</h2>
87+
<p>Click the buttons below to test onclick handlers:</p>
88+
89+
<!-- Test inline onclick attribute -->
90+
<button id="btn1" onclick="logEvent('btn1', 'Inline onclick attribute handler executed')">
91+
Click me (inline handler)
92+
</button>
93+
94+
<!-- Test property assignment -->
95+
<button id="btn2">
96+
Click me (property handler)
97+
</button>
98+
99+
<!-- Test both attribute and property -->
100+
<button id="btn3" onclick="logEvent('btn3', 'Inline handler (should be overridden)')">
101+
Click me (both handlers)
102+
</button>
103+
104+
<button id="runAllTests" onclick="runAutomatedTests()" style="background-color: #87CEEB;">
105+
Run Automated Tests
106+
</button>
107+
</div>
108+
109+
<div class="test-section">
110+
<h2>Test Results</h2>
111+
<div id="results" class="results">
112+
<div class="status" id="status">Ready to test...</div>
113+
</div>
114+
</div>
115+
116+
<script>
117+
let testCount = 0;
118+
let passedTests = 0;
119+
let failedTests = 0;
120+
121+
function addLog(message, type = 'info') {
122+
const resultsDiv = document.getElementById('results');
123+
const logEntry = document.createElement('div');
124+
logEntry.className = `log-entry ${type}`;
125+
logEntry.textContent = `${new Date().toLocaleTimeString()}: ${message}`;
126+
resultsDiv.appendChild(logEntry);
127+
resultsDiv.scrollTop = resultsDiv.scrollHeight;
128+
129+
// Also log to console for debugging
130+
console.log(message);
131+
}
132+
133+
function updateStatus() {
134+
const statusDiv = document.getElementById('status');
135+
statusDiv.textContent = `Tests: ${testCount} | Passed: ${passedTests} | Failed: ${failedTests}`;
136+
}
137+
138+
function logEvent(name, message) {
139+
addLog(`✅ SUCCESS: ${name} - ${message}`, 'success');
140+
141+
// Visual feedback
142+
const button = document.getElementById(name);
143+
if (button) {
144+
button.classList.add('test-button-clicked');
145+
setTimeout(() => button.classList.remove('test-button-clicked'), 1000);
146+
}
147+
148+
passedTests++;
149+
updateStatus();
150+
}
151+
152+
function runTest(testName, testFunction) {
153+
testCount++;
154+
addLog(`🔍 RUNNING: ${testName}`, 'info');
155+
156+
try {
157+
testFunction();
158+
addLog(`✅ PASSED: ${testName}`, 'success');
159+
passedTests++;
160+
} catch (error) {
161+
addLog(`❌ FAILED: ${testName} - ${error.message}`, 'error');
162+
failedTests++;
163+
}
164+
165+
updateStatus();
166+
}
167+
168+
function runAutomatedTests() {
169+
// Clear previous results
170+
document.getElementById('results').innerHTML = '<div class="status" id="status">Running tests...</div>';
171+
testCount = 0;
172+
passedTests = 0;
173+
failedTests = 0;
174+
175+
addLog('🚀 Starting OnClick Handler Tests...', 'info');
176+
177+
// Test 1: Property assignment
178+
runTest('Property Assignment', () => {
179+
const btn2 = document.getElementById('btn2');
180+
if (!btn2) throw new Error('Button 2 not found');
181+
182+
btn2.onclick = function() { logEvent('btn2', 'Property assignment handler executed'); };
183+
addLog('✓ Set btn2.onclick property', 'info');
184+
185+
if (typeof btn2.onclick !== 'function') {
186+
throw new Error('onclick property should be a function');
187+
}
188+
});
189+
190+
// Test 2: Property override attribute
191+
runTest('Property Override Attribute', () => {
192+
const btn3 = document.getElementById('btn3');
193+
if (!btn3) throw new Error('Button 3 not found');
194+
195+
btn3.onclick = function() { logEvent('btn3', 'Property handler executed (overrode attribute)'); };
196+
addLog('✓ Set btn3.onclick property to override attribute', 'info');
197+
198+
if (typeof btn3.onclick !== 'function') {
199+
throw new Error('onclick property should be a function');
200+
}
201+
});
202+
203+
// Test 3: Getter verification
204+
runTest('Onclick Getter Verification', () => {
205+
const btn1 = document.getElementById('btn1');
206+
const btn2 = document.getElementById('btn2');
207+
const btn3 = document.getElementById('btn3');
208+
209+
if (btn1) {
210+
addLog(`✓ btn1.onclick type: ${typeof btn1.onclick}`, 'info');
211+
}
212+
if (btn2) {
213+
addLog(`✓ btn2.onclick type: ${typeof btn2.onclick}`, 'info');
214+
}
215+
if (btn3) {
216+
addLog(`✓ btn3.onclick type: ${typeof btn3.onclick}`, 'info');
217+
}
218+
});
219+
220+
// Test 4: Programmatic clicks
221+
setTimeout(() => {
222+
addLog('🖱️ Testing programmatic clicks...', 'info');
223+
224+
runTest('Programmatic Click - Button 1', () => {
225+
const btn1 = document.getElementById('btn1');
226+
if (!btn1) throw new Error('Button 1 not found');
227+
228+
addLog('🖱️ Clicking btn1 programmatically...', 'info');
229+
btn1.click();
230+
});
231+
232+
setTimeout(() => {
233+
runTest('Programmatic Click - Button 2', () => {
234+
const btn2 = document.getElementById('btn2');
235+
if (!btn2) throw new Error('Button 2 not found');
236+
237+
addLog('🖱️ Clicking btn2 programmatically...', 'info');
238+
btn2.click();
239+
});
240+
}, 500);
241+
242+
setTimeout(() => {
243+
runTest('Programmatic Click - Button 3', () => {
244+
const btn3 = document.getElementById('btn3');
245+
if (!btn3) throw new Error('Button 3 not found');
246+
247+
addLog('🖱️ Clicking btn3 programmatically...', 'info');
248+
btn3.click();
249+
});
250+
251+
setTimeout(() => {
252+
addLog('🎉 OnClick Handler Tests Completed!', 'success');
253+
updateStatus();
254+
}, 500);
255+
}, 1000);
256+
}, 1000);
257+
}
258+
259+
// Initialize the test page
260+
document.addEventListener('DOMContentLoaded', () => {
261+
addLog('📄 Page loaded successfully', 'info');
262+
addLog('👆 Click buttons above to test onclick handlers', 'info');
263+
addLog('🤖 Or click "Run Automated Tests" for comprehensive testing', 'info');
264+
updateStatus();
265+
});
266+
</script>
267+
</body>
268+
269+
</html>

‎src/bindings/dom/html_element-inl.hpp‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,13 @@ namespace dombinding
1212
auto props = ElementBase<ObjectType, HTMLElementType>::GetClassProperties(env);
1313
auto added = vector<Napi::ClassPropertyDescriptor<ObjectType>>(
1414
{
15+
// Fields
1516
T::InstanceAccessor("dataset", &T::DatasetGetter, nullptr),
1617
T::InstanceAccessor("style", &T::StyleGetter, nullptr),
18+
// Methods
19+
T::InstanceMethod("blur", &T::Blur),
20+
T::InstanceMethod("click", &T::Click),
21+
T::InstanceMethod("focus", &T::Focus),
1722
});
1823
props.insert(props.end(), added.begin(), added.end());
1924
return props;
@@ -35,4 +40,34 @@ namespace dombinding
3540
Napi::HandleScope scope(env);
3641
return cssombinding::CSSStyleDeclaration::NewInstance(env, this->node->styleRef());
3742
}
43+
44+
template <typename ObjectType, typename HTMLElementType>
45+
Napi::Value HTMLElementBase<ObjectType, HTMLElementType>::Blur(const Napi::CallbackInfo &info)
46+
{
47+
Napi::Env env = info.Env();
48+
auto htmlElement = dynamic_pointer_cast<dom::HTMLElement>(this->node);
49+
assert(htmlElement != nullptr && "The node is not an HTMLElement.");
50+
htmlElement->blur();
51+
return env.Undefined();
52+
}
53+
54+
template <typename ObjectType, typename HTMLElementType>
55+
Napi::Value HTMLElementBase<ObjectType, HTMLElementType>::Click(const Napi::CallbackInfo &info)
56+
{
57+
Napi::Env env = info.Env();
58+
auto htmlElement = dynamic_pointer_cast<dom::HTMLElement>(this->node);
59+
assert(htmlElement != nullptr && "The node is not an HTMLElement.");
60+
htmlElement->click();
61+
return env.Undefined();
62+
}
63+
64+
template <typename ObjectType, typename HTMLElementType>
65+
Napi::Value HTMLElementBase<ObjectType, HTMLElementType>::Focus(const Napi::CallbackInfo &info)
66+
{
67+
Napi::Env env = info.Env();
68+
auto htmlElement = dynamic_pointer_cast<dom::HTMLElement>(this->node);
69+
assert(htmlElement != nullptr && "The node is not an HTMLElement.");
70+
htmlElement->focus();
71+
return env.Undefined();
72+
}
3873
}

‎src/bindings/dom/html_element.hpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ namespace dombinding
6060
public:
6161
Napi::Value DatasetGetter(const Napi::CallbackInfo &info);
6262
Napi::Value StyleGetter(const Napi::CallbackInfo &info);
63+
64+
Napi::Value Blur(const Napi::CallbackInfo &info);
65+
Napi::Value Click(const Napi::CallbackInfo &info);
66+
Napi::Value Focus(const Napi::CallbackInfo &info);
6367
};
6468

6569
class HTMLElement : public HTMLElementBase<HTMLElement, dom::HTMLElement>

‎src/client/dom/dom_scripting.cpp‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ namespace dom
247247
: isolate(v8::Isolate::GetCurrent())
248248
, runtimeContext(runtimeContext)
249249
{
250+
assert(isolate != nullptr && "Failed to get the current V8 isolate.");
250251
}
251252

252253
void DOMScriptingContext::enableDynamicImport()
@@ -625,8 +626,12 @@ namespace dom
625626
bool DOMScriptingContext::compile(shared_ptr<DOMScript> script, const string &source, bool isTypeScript)
626627
{
627628
assert(isContextInitialized);
628-
auto context = v8ContextStore.Get(isolate);
629+
630+
// Create a handle scope to store the context, that is a `Local<Context>`.
629631
v8::Isolate::Scope isolateScope(isolate);
632+
v8::HandleScope handleScope(isolate);
633+
634+
v8::Local<v8::Context> context = v8ContextStore.Get(isolate);
630635
v8::Context::Scope contextScope(context);
631636

632637
if (!script->compile(isolate, source, isTypeScript))
@@ -645,8 +650,10 @@ namespace dom
645650
bool DOMScriptingContext::compileAsSyntheticModule(shared_ptr<DOMModule> module, SyntheticModuleType type, const void *sourceData, size_t sourceByteLength)
646651
{
647652
assert(isContextInitialized);
648-
auto context = v8ContextStore.Get(isolate);
649653
v8::Isolate::Scope isolateScope(isolate);
654+
v8::HandleScope handleScope(isolate);
655+
656+
auto context = v8ContextStore.Get(isolate);
650657
v8::Context::Scope contextScope(context);
651658

652659
if (!module->compileAsSyntheticModule(isolate, type, sourceData, sourceByteLength))
@@ -660,8 +667,10 @@ namespace dom
660667
void DOMScriptingContext::evaluate(shared_ptr<DOMScript> script)
661668
{
662669
assert(isContextInitialized);
663-
auto context = v8ContextStore.Get(isolate);
664670
v8::Isolate::Scope isolateScope(isolate);
671+
v8::HandleScope handleScope(isolate);
672+
673+
v8::Local<v8::Context> context = v8ContextStore.Get(isolate);
665674
v8::Context::Scope contextScope(context);
666675
{
667676
script->evaluate(isolate);

0 commit comments

Comments
 (0)