Skip to content

Remove non-existent unload event handler from webagg image object - #32402

Closed
kokhlo wants to merge 1 commit into
matplotlib:mainfrom
kokhlo:fix/webagg-remove-nonexistent-unload-handler
Closed

kokhlo wants to merge 1 commit into
matplotlib:mainfrom
kokhlo:fix/webagg-remove-nonexistent-unload-handler

Conversation

@kokhlo

@kokhlo kokhlo commented Sep 26, 2026

Copy link
Copy Markdown

Summary

The webagg frontend (lib/matplotlib/backends/web_backend/js/mpl.js) assigned an onunload handler to the rendering Image element that closed the websocket. No unload event exists on HTMLImageElement or any of its ancestors (HTMLElement, Element, Node) — per the MDN docs it only exists on the global Window interface. The assignment was a plain expando property that no browser ever invoked, so the handler has been dead code since it was introduced.

Removing it outright (rather than migrating to pagehide/visibilitychange) is behavior-neutral because the real cleanup paths are untouched:

  • On page teardown the browser closes all open WebSocket connections; the tornado side already handles that via WebSocketHandler.on_close → FigureManagerWebAgg.remove_web_socket (backend_webagg.py).
  • For nbagg, the comm is closed through Jupyter's own comm lifecycle (comm.on_close in backend_nbagg.py), independent of this handler.

Testing

  • node --check js/mpl.js — syntax OK
  • grep -c onunload js/*.js — 0 occurrences remain
  • Diff is a pure 4-line deletion; no other references to the handler exist in the repo
  • The JS is shipped unbundled (meson install_subdir('js'), served via StaticFileHandler), so no build step applies

Fixes #32393

The webagg frontend assigned an onunload handler to the rendering
Image element, but no unload event exists on HTMLImageElement or any
of its ancestors, so the handler was never invoked by any browser.
Websocket cleanup on page teardown is already handled by the browser
closing the connection and the server-side on_close path.
@github-actions

Copy link
Copy Markdown

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process.

You can also join us on discourse chat for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide.
Please let us know if (and how) you use AI, it will help us give you better feedback on your PR.

We strive to be a welcoming and open project. Please follow our Code of Conduct.

@github-actions github-actions Bot added first-contribution GUI: webagg AgentScan: automated AgentScan classified an account as automated. labels Sep 26, 2026
@iccir

iccir commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Closing this PR and blocking the account for one week per our AI policy.

The account in question has opened several PRs across multiple repositories in a rapid fashion. Additionally, AgentScan detected automated activity.

@iccir iccir closed this Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AgentScan: automated AgentScan classified an account as automated. first-contribution GUI: webagg

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: webagg backend uses non-existent unload event

2 participants