Page MenuHomePhabricator

Some SelectWidgets don't disconnect keydown listeners when detached
Open, Needs TriagePublic

Description

Some SelectWidgets (e.g.RadioSelectWidget) add document keydown listeners when focused, and removed them if blurred. However if they are detached from the DOM while focussed, the blur event never fire, keeping the object in memory, and causing the keypress event to fire on the next keystroke.

An example from EditCheck:

  1. Trigger a check that has a survey, e.g. PasteCheck or ToneCheck
  2. Start the survey, as select a radio option:
    image.png (324×367 px, 28 KB)
  3. Press back to hide the form
  4. Focus the VE surface and try to use the arrow keys. Observe that arrow keys do not move your cursor.

Event Timeline

Change #1184729 had a related patch set uploaded (by Esanders; author: Esanders):

[oojs/ui@master] SelectWidget: Use focusin/out instead of focus/blur

https://gerrit.wikimedia.org/r/1184729

The only reason this doesn't happen more often in VE is because most of our form controls are in OO.ui.Windows, and windows manually move the focus before closing, which triggers a 'blur' and unbinds the keydown handlers.

I'm kind of surprised that clicking on the "submit" isn't enough to blur the selectwidget. I suppose that the buttonelement canceling the click event and handling it manually must be enough to stop that.

Change #1184729 merged by jenkins-bot:

[oojs/ui@master] SelectWidget: Use focusin/out instead of focus/blur

https://gerrit.wikimedia.org/r/1184729

Change #1219646 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/core@master] Update OOUI to v0.53.1

https://gerrit.wikimedia.org/r/1219646

Change #1219646 merged by jenkins-bot:

[mediawiki/core@master] Update OOUI to v0.53.1

https://gerrit.wikimedia.org/r/1219646

The above patch works when the element is removed from the DOM by any native method, but jQuery's #remove will unbind all events before removing the node, including the focusout event we are expecting to run...

Change #1219850 had a related patch set uploaded (by Esanders; author: Esanders):

[oojs/ui@master] SelectWidget: Use native event binding for focusin/out

https://gerrit.wikimedia.org/r/1219850

Change #1219850 merged by jenkins-bot:

[oojs/ui@master] SelectWidget: Use native event binding for focusin/out

https://gerrit.wikimedia.org/r/1219850

Esanders renamed this task from Some SelectWidgets don't disconnect keydown listeners when detacted to Some SelectWidgets don't disconnect keydown listeners when detached.Jan 5 2026, 3:13 PM

This just needs another OOUI release.

ppelberg subscribed.

@Esanders to pull that back onto the sprint board for QA once OOUI release goes out.

Change #1275892 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/core@master] Upgrade OOUI from v0.53.1 to v0.53.2

https://gerrit.wikimedia.org/r/1275892

Change #1275892 merged by jenkins-bot:

[mediawiki/core@master] Upgrade OOUI from v0.53.1 to v0.53.2

https://gerrit.wikimedia.org/r/1275892

Change #1277108 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/VisualEditor@master] EditCheckActionWidget: Remove workaround for T403828

https://gerrit.wikimedia.org/r/1277108

Change #1277108 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] EditCheckActionWidget: Remove workaround for T403828

https://gerrit.wikimedia.org/r/1277108