Skip to content

Conversation

@DrJKL
Copy link
Contributor

@DrJKL DrJKL commented Sep 29, 2025

Summary

Fixes the laggy textarea select noted in Discord.

Changes

  • What: Fixes the case where a DomWidget that was repeatedly made visible accumulated as many event listeners as times it was toggled on.

┆Issue is synchronized with this Notion page by Unito

@DrJKL DrJKL requested a review from webfiltered September 29, 2025 08:28
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Sep 29, 2025
@github-actions
Copy link

github-actions bot commented Sep 29, 2025

🎭 Playwright Test Results

⚠️ Tests passed with flaky tests

⏰ Completed at: 09/29/2025, 08:38:28 AM UTC

📈 Summary

  • Total Tests: 479
  • Passed: 448 ✅
  • Failed: 0
  • Flaky: 2 ⚠️
  • Skipped: 29 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 440 / ❌ 0 / ⚠️ 2 / ⏭️ 29
  • chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • mobile-chrome: View Report • ✅ 5 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

Copy link
Contributor

@webfiltered webfiltered left a comment

Choose a reason for hiding this comment

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

I would recommend testing this in normal usage - undo, redo, add/remove, subgraph in / out, workflow swaps, and in a decent size workflow (pan / zoom around and make some changes).

I can't recall the gotchas on this atm. I believe it was refactored during subgraph work (simpler?).

@DrJKL
Copy link
Contributor Author

DrJKL commented Sep 29, 2025

I would recommend testing this in normal usage - undo, redo, add/remove, subgraph in / out, workflow swaps, and in a decent size workflow (pan / zoom around and make some changes).

I can't recall the gotchas on this atm. I believe it was refactored during subgraph work (simpler?).

Okay, did a decent number of checks, nothing seemed to be broken.

@christian-byrne
Copy link
Contributor

christian-byrne commented Sep 29, 2025

IIRC, I had edited this code for subgraphs. I just tested all the cases for which I was covering at the time -- everything still works per expectation. For the DOM widget promotion, there are also browser tests giving us coverage. It all looks good to me.

Copy link
Collaborator

@AustinMroz AustinMroz left a comment

Choose a reason for hiding this comment

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

I'm also not seeing any issues, either from code or tests. The fix seems comprehensive. Thanks for chasing this one down!

@AustinMroz AustinMroz merged commit dd93ac4 into main Sep 29, 2025
26 checks passed
@AustinMroz AustinMroz deleted the drjkl/fix/dom-widget-listeners branch September 29, 2025 18:52
@AustinMroz AustinMroz added needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch 1.27 labels Sep 29, 2025
github-actions bot pushed a commit that referenced this pull request Sep 29, 2025
## Summary

Fixes the laggy textarea select noted in Discord.

## Changes

- **What**: Fixes the case where a DomWidget that was repeatedly made
visible accumulated as many event listeners as times it was toggled on.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5846-fix-Only-add-the-listeners-for-DomWidget-components-once-27d6d73d3650818382e0d0c565fc862a)
by [Unito](https://www.unito.io)
@comfy-pr-bot
Copy link
Member

@DrJKL Successfully backported to #5849

christian-byrne pushed a commit that referenced this pull request Sep 30, 2025
…once (#5849)

Backport of #5846 to `core/1.27`

Automatically created by backport workflow.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5849-backport-1-27-fix-Only-add-the-listeners-for-DomWidget-components-once-27d6d73d365081438673f970b0508032)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Alexander Brown <[email protected]>
Co-authored-by: github-actions <[email protected]>
christian-byrne pushed a commit that referenced this pull request Oct 6, 2025
## Summary

Fixes the laggy textarea select noted in Discord.

## Changes

- **What**: Fixes the case where a DomWidget that was repeatedly made
visible accumulated as many event listeners as times it was toggled on.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5846-fix-Only-add-the-listeners-for-DomWidget-components-once-27d6d73d3650818382e0d0c565fc862a)
by [Unito](https://www.unito.io)
arjansingh pushed a commit that referenced this pull request Oct 7, 2025
## Summary

Fixes the laggy textarea select noted in Discord.

## Changes

- **What**: Fixes the case where a DomWidget that was repeatedly made
visible accumulated as many event listeners as times it was toggled on.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5846-fix-Only-add-the-listeners-for-DomWidget-components-once-27d6d73d3650818382e0d0c565fc862a)
by [Unito](https://www.unito.io)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.27 needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants