Skip to content

Fix example in Web/API/Document_Object_Model#43300

Open
whxvd wants to merge 1 commit intomdn:mainfrom
whxvd:1
Open

Fix example in Web/API/Document_Object_Model#43300
whxvd wants to merge 1 commit intomdn:mainfrom
whxvd:1

Conversation

@whxvd
Copy link
Contributor

@whxvd whxvd commented Mar 1, 2026

The last example (Displaying event object properties) is meant to "to display all the properties of the onload event object and their values". But the example did not register the appropriate event handler; instead it had an erroneous last line.

@whxvd whxvd requested a review from a team as a code owner March 1, 2026 06:15
@whxvd whxvd requested review from dipikabh and removed request for a team March 1, 2026 06:15
@github-actions github-actions bot added Content:WebAPI Web API docs size/xs [PR only] 0-5 LoC changed labels Mar 1, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

Preview URLs (1 page)

(comment last updated: 2026-03-07 07:37:20)

}

showEventProperties(event);
onload = showEventProperties;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onload = showEventProperties;
window.addEventListener("click", showEventProperties);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, that would introduce three changes:

  1. window.addEventListener instead of the onload property. That is fine. After all, addEventListener is the recommended way. I had used the onload property, because currently many pages on MDN use them, and I had thought that for brevity or in examples, event properties may be preferred.

  2. Make the global object explicit (window.). That's fine, too.

  3. "click" instead of "load". That would not fit the preceding text that explains the example. IMO, that text and the load event are fine. I am in favor of sticking to the load event.

Copy link
Member

Choose a reason for hiding this comment

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

3. "click" instead of "load". That would not fit the preceding text that explains the example. IMO, that text and the load event are fine. I am in favor of sticking to the load event.

I prefer click because it contains non-trivial properties such as coordinates, plus it can fire many times. load events are also generally frowned upon in MDN because code examples used to abuse them while top-level code works just fine, so sometimes I go around and remove unnecessary load event listeners. It's best if we can avoid them where we can to make the auditing easier. You can update the preceding text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, now the click event is used.

The last example (Displaying event object properties) had an erroneous line. The event handler was not actually registered.

Apart from that, now the example uses the click event instead of the load event.

The table now has a little margin at the top, so that several tables (after several click events) can be discerned more easily.
@github-actions github-actions bot added size/s [PR only] 6-50 LoC changed and removed size/xs [PR only] 0-5 LoC changed labels Mar 7, 2026
@whxvd
Copy link
Contributor Author

whxvd commented Mar 7, 2026

The text had a "double domxref" to Window.load_event and event. I found that a bit strange and switched to the simplest form. The documentation for Event is easily reacheble from Element.click_event. It does not need an extra link.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Content:WebAPI Web API docs size/s [PR only] 6-50 LoC changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants