Change Details

I've experienced an endless loop when selecting an item in the browser in a DropdownInputWidget that contains a MenuSectionOptionWidget. The loop repeats * selectItem (SelectWidget.js:755) * setValue (DropdownInputWidget.js:104) * onMenuSelect (DropdownInputWidget.js:92) * emit (jQuery) Looking at the code, I found one possible cause of this: in `SelectWidget.selectItem`, the check is `this.item[ i ].isSelected() === selected`. However, for `MenuSectionOptionWidget` items, the `selected` property is absent, leading to the variable `changed` in `selectItem` to be always set (because `false !== undefined`), triggering the "select" event emission. My suggestion for a fix would be to set `this.selected` to `false` in the `MenuSectionOptionWidget`. Relying on the parent constructors wouldn't work since they call `this.setSelected` in the constructor, which skips setting the `selected` property when the static `selectable` property is set. Another possible fix would be to change the `isSelected` method to `return !!this.selected`. However, the double negation could have performance impacts. My colleagues could not reproduce my error and it only occurs on my local machine (latest mediawiki master). My proposed change fixes it. On the current Wikipedia the error is also absent. Nevertheless, I would like to point out the potential for this logic error in the code. For me, the error occurs when selecting an item for "Filetype" on the "Search" page, with the "AdvancedSearch" extension active.

I've experienced an endless loop when selecting an item in the browser in a DropdownInputWidget that contains a MenuSectionOptionWidget. The loop repeats * selectItem (SelectWidget.js:755) * setValue (DropdownInputWidget.js:104) * onMenuSelect (DropdownInputWidget.js:92) * emit (jQuery) Looking at the code, I found one possible cause of this: in `SelectWidget.selectItem`, the check is `this.item[ i ].isSelected() === selected`. However, for `MenuSectionOptionWidget` items, the `selected` property is absent, leading to the variable `changed` in `selectItem` to be always set (because `false !== undefined`), triggering the "select" event emission. My suggestion for a fix would be to set `this.selected` to `false` in the `MenuSectionOptionWidget` constructor. Relying on the parent constructors wouldn't work since they call `this.setSelected` in the constructor, which skips setting the `selected` property when the static `selectable` property is set. Another possible fix would be to change the `isSelected` method to `return !!this.selected`. However, the double negation could have performance impacts. My colleagues could not reproduce my error by manually user testing. On the current Wikipedia the error is also absent. But several pull requests for AdvancedSearch, with changes unrelated to the dropdown selection, have started to fail: * https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AdvancedSearch/+/496414 * https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AdvancedSearch/+/495922 * https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AdvancedSearch/+/496174 I can provide a patch for `MenuSectionOptionWidget`, but would also be interested in the "root cause" - the OOUI change that triggered this. So far I did not find any.

I've experienced an endless loop when selecting an item in the browser in a DropdownInputWidget that contains a MenuSectionOptionWidget. The loop repeats * selectItem (SelectWidget.js:755) * setValue (DropdownInputWidget.js:104) * onMenuSelect (DropdownInputWidget.js:92) * emit (jQuery) Looking at the code, I found one possible cause of this: in `SelectWidget.selectItem`, the check is `this.item[ i ].isSelected() === selected`. However, for `MenuSectionOptionWidget` items, the `selected` property is absent, leading to the variable `changed` in `selectItem` to be always set (because `false !== undefined`), triggering the "select" event emission. My suggestion for a fix would be to set `this.selected` to `false` in the `MenuSectionOptionWidget` constructor. Relying on the parent constructors wouldn't work since they call `this.setSelected` in the constructor, which skips setting the `selected` property when the static `selectable` property is set. Another possible fix would be to change the `isSelected` method to `return !!this.selected`. However, the double negation could have performance impacts. My colleagues could not reproduce my error and it only occurs on my local machine (latest mediawiki master)by manually user testing. My proposed change fixes itOn the current Wikipedia the error is also absent. On the current Wikipedia the error is also absent.But several pull requests for AdvancedSearch, Neverthelesswith changes unrelated to the dropdown selection, I would like to point out the potential for this logic error in the code. For mehave started to fail: * https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AdvancedSearch/+/496414 * https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AdvancedSearch/+/495922 * https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AdvancedSearch/+/496174 I can provide a patch for `MenuSectionOptionWidget`, the error occurs when selecting an item for "Filetype" onbut would also be interested in the "root cause" - the "Search" page,OOUI change that triggered this. with the "AdvancedSearch" extension activeSo far I did not find any.