Page MenuHomePhabricator

Form input resets existing value when not in available data of dropdown autocomplete (ExternalData)
Closed, ResolvedPublicBUG REPORT

Description

List of steps to reproduce (step by step, including full links if applicable):

  • Edit a form that has input type dropdown and autocomplete data from ExternalData
  • Force the form to store a value that does not exist in the data, e.g. xxxx
  • Save the form. Notice that xxxx is in the page
  • Click edit with the form again. Look closely at the field. It starts with xxxx and then when javascript select2 dropdown kicks in it is getting deleted.

What happens?:
While the system allows custom values they get reset on the second edit during reload.

What should have happened instead?:
The system should either allow or disallow custom values, preferably by a configuration variable.

  1. If custom values are allowed the system shouldn't reset the input value on formedit
  2. If custom values are not allowed then select2 shouldn't allow to be entered in the first place.

There is a comment in the code that I duplicated (along with the code) in the patch below. The patch handles the first case.

It could probably be reworked a bit and move the custom value handling outside the if branch. The existing code already handles the same way the non ExternalData case. I am just adding support for this dubious feature in the ExternalData case so they can be consistent.

--- mediawiki.orig/extensions/PageForms/libs/ext.pf.select2.combobox.js 2021-04-28 13:05:07.000000000 -0400
+++ mediawiki/extensions/PageForms/libs/ext.pf.select2.combobox.js      2021-05-19 05:01:11.218615823 -0400
@@ -129,6 +129,21 @@
                                if ( wgPageFormsEDSettings[name].title !== undefined && wgPageFormsEDSettings[name].title !== "" ) {
                                        data.title = edgValues[wgPageFormsEDSettings[name].title];
                                        if ( data.title !== undefined && data.title !== null ) {
+                                               // Add the current value to the list of allowed
+                                               // values, if it's not there already, so that the
+                                               // combobox won't show up as blank.
+                                               // Should this be done even if "existing values
+                                               // only" is specified"? For now, yes - based on
+                                               // the "principle of least astonishment", it's
+                                               // generally better for the form to not wipe out
+                                               // existing content when possible. However,
+                                               // there's also an argument the other way.
+                                               var curValue = $('#' + this.id).attr('value');
+                                               if ( curValue !== '' && curValue !== undefined && !Object.keys(data).includes( curValue ) ) {
+                                                       values.push({
+                                                               id: curValue, text: curValue
+                                                       });
+                                               }
                                                i = 0;
                                                data.title.forEach(function() {
                                                        values.push({

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc:

  • mediawiki 1.35.2 from git
  • PageForms: 5.2.1
  • ExternalData 2.4 from git

Thanks

Event Timeline

Yaron_Koren claimed this task.
Yaron_Koren subscribed.

I believe that this is no longer an issue, now that the combobox input type (referred to in the task name as "dropdown") has switched to using OOUI instead of Select2.