Page MenuHomePhabricator

Do not save unused (or deliberately removed) suggested parameters when inserting or editing transclusions
Closed, ResolvedPublic1 Estimated Story Points

Event Timeline

Jdforrester-WMF raised the priority of this task from to Medium.
Jdforrester-WMF updated the task description. (Show Details)
Jdforrester-WMF added a subscriber: Jdforrester-WMF.

Can this actually be done without potential side-effects? Handling Parameters suggests that there's a difference between an empty parameter and a not-provided-at-all parameter, in terms of whether the default value gets used... and we can't tell the difference between a deliberate empty-string and the user just not caring about that parameter.

Deskana set the point value for this task to 1.Oct 27 2017, 3:13 PM
Krinkle renamed this task from When inserting a new transclusion, drop empty suggested (but not required) parameters to Do not save unused (or deliberately removed) suggested parameters when inserting or editing transclusions.Dec 2 2017, 10:41 PM

Regarding "empty" vs "non-existent", this is true, but applies in both directions. When a template suggests a parameter "foo" and expects a non-empty value, one could argue VisualEditor should not save them by default with an empty value.

In this state, I believe it might be better to remove support for the "suggested" attribute and simply treat them as all other optional attributes that can be inserted via "Add more". Until we've developed an approach that can't damage content.

Based on the TemplateData specification, I think the below approach could work, but it's just an idea:

  1. Easy first steps:
    • Remove existing handling, turn them back into regular optional parameters, like how it was before.
    • Have "suggested" parameters sort higher in the list shown after clicking "Add more".
  2. Future development:
    • Introduce a new kind of input field that is similar to a "slug" paragraph placeholder. It is present in the dialog with its name and documentation, but its input field is absent or muted in a way that (both visually and by other accessible means) indicates it is not currently an applicable parameter. If it is clicked on, it becomes a real field with an empty/default value. If it is removed via the trash can, it either becomes a suggested placeholder again or (alternatively) removed from view (to be added back via "Add more"). When saving, only the real parameters are submitted.

I think the approach suggested by @Krinkle above may be overkill.

The most straightforward path to the goal would seem to be to keep track of parameters which are displayed only due to being "suggested" (i.e., not because a user actively asked for it), and drop them before saving if no value was entered.

If a user actively asks for a parameter we can't know whether an empty instance of it has significance; but a parameter that's "suggested" and empty will never have significance or that value would have simply been hardcoded in the template.

Note also that "suggested" is in practice partly being used for "I use this param often and want convenient access to it". A per user feature that remembers the user's most frequently used parameters for each template and makes them easily available would reduce the perceived need for "suggested" parameters. For this purpose I could see the kind of "Schrödinger's text field" @Krinkle describes: your five most used params are displayed by default, but in a proto/placeholder type state, only turning real when instantiated by entering text into it.

I agree it's annoying that VE inserts empty parameters which have not been touched at all by the user. It's unnecessary baggage in the page text, and complicates the diff. Ideally VE should produce a diff similar to whatever the equivalent non-Visual ("human") edit would have produced: don't change whitespace, parameter order, or add any superfluous parameters.

Change 641218 had a related patch set uploaded (by TK-999; owner: TK-999):
[mediawiki/extensions/VisualEditor@master] ve.dm.MWTemplateModel: Don't add spurious empty parameters

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

Looks like the old Wikia-WMF VisualEditor implementation is handling this case already, so I have ported the relevant logic from https://github.com/Wikia/app/pull/6450/commits/858eaa928dda808999d9c3f0e253dcde582b901e#diff-499ee0316d78ea3542a87f1412b1db0adf23bf227669ad50495aa7ef7988b6a0R74 and submitted a patch.

Let's do this and see what happens.

With the patch, any newly added template parameters that are left empty are removed completely. This applies to both automatically added "suggested" parameters, and parameters manually added in the dialog. Parameters that are specified but empty in wikitext are not removed, and neither are parameters that already existed and are emptied.

As noted earlier, there's a difference in wikitext between an empty parameter and a not-provided-at-all parameter, but I don't expect this to come up often in practice. If we receive a sudden stream of bug reports saying that it's impossible to add some parameters, we should undo this change and think about other options.

I'm in favor of the patch -- it probably represents more what people would expect to have happen.

If we receive a sudden stream of bug reports saying that it's impossible to add some parameters, we should undo this change and think about other options.

This patch plus another little field-toggle like the wikitext button that opts in/out of keeping the empty-field would probably cover everything, if that's needed.

Change 641218 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.dm.MWTemplateModel: Don't add spurious empty parameters

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

Let's do this and see what happens.

With the patch, any newly added template parameters that are left empty are removed completely. This applies to both automatically added "suggested" parameters, and parameters manually added in the dialog. Parameters that are specified but empty in wikitext are not removed, and neither are parameters that already existed and are emptied.

As noted earlier, there's a difference in wikitext between an empty parameter and a not-provided-at-all parameter, but I don't expect this to come up often in practice. If we receive a sudden stream of bug reports saying that it's impossible to add some parameters, we should undo this change and think about other options.

I like the simple and unobtrusive compromise this makes. Thank you.

ppelberg claimed this task.

… there's a difference in wikitext between an empty parameter and a not-provided-at-all parameter, …

Just noting for future reference, there are some prominent examples of defined-but-empty parameters that spring to mind.

On enwp there's {{category handler}}, that is also widely copied across projects, where an empty parameter means "don't add a category in this namespace". On English Wikisource there is {{page break}} which by default outputs a visible text label which can be suppressed by giving an empty label parameter.

I am guessing the use case "suppress a default value" is a common one for empty-but-defined parameters, and would be surprised if use of it across the projects is not larger than negligible (in "number of templates using it * number of uses of that template * fraction of uses that rely on the empty param" terms).

Of course, I agree these are unlikely currently to come up much in practice: {{category handler}} is a utility template for making other templates and unlikely to be used with VE, and Visual Editor currently does not actually support Wikisource so this would be just one more thing that's broken there. But it's worth keeping in mind longer term: so long as MW templates support empty-but-defined params they will be used and VE will eventually have to support them fully. Or empty-but-defined params could be deprecated in MW, which would hurt a bit short term but make for a cleaner model long term.

I filed a new task about better support for parameters that are intentionally empty: T280078.