Page MenuHomePhabricator

Users document template parameter "" instead of "1" because of incomplete validation
Closed, ResolvedPublicBUG REPORT

Description

Via T64417#8746816 we realized that it's possible to have parameters with no name in the TemplateData documentation, e.g. "params": { "": {} }. This breaks VisualEditor and an unknown number of other consumers, possibly including ContentTranslation.

As of now I can find 6 instances on enwiki. Most of them seem to be mistakes, e.g. the {{navbar}} documentation is clearly meant to document the parameter "1". At least one, namely {{stub}}, appear to intentionally trigger a specific sequence of bugs in an attempt to make the "Add undocumented parameter" button unreachable. The reason the button disappears is an unspecified combination of:

  • TemplateData misses a validation step. This affects both the validation when a page is saved as well as what the API returns. Both use the same validation.
  • The VisualEditor template dialog uses the empty string to represent a currently unnamed parameter that is already visible via the AddParameter page in the BookletLayout and the sidebar, but not named yet. This implies there can only be one unnamed parameter at a time. This is an intentional restriction.
  • Whenever an empty string appears in the list of parameters the specified behavior should be to show said AddParameter page.
  • Unfortunately the code that initializes the dialog doesn't check if an AddParameter page already exists (that would be using UI state as data, which is bad practice). Instead it checks the data model for an empty string and doesn't add a new AddParameter page, believing there is already one.

The dialog runs into more user-facing issues because of this. For example, it's still possible to add parameters by pressing Ctrl+Shift+D. This is an intentional power-user feature. The problem is that the AddParameter page stays at the top of the parameter list when it should be at the bottom. Some places in the code count the empty parameter, others don't, causing sync issues with unknown consequences.

Possible steps forward:

  • Fix VisualEditor and make it behave consistent.
  • Make the TemplateData validation more strict. Note this will make the API fail for these template. Their documentation will be unreachable until it is fixed.
  • Should we go ahead and manually fix the broken templates we are aware of? Do we have a good way to ask the community to do this?

Event Timeline

For en:{{Stub}}, given the issuing JSON has been added recently by an anonymous user without any discussion, I’m not sure it is intentional.

Change 907920 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/VisualEditor@master] Fix template dialog when TemplateData contains empty parameter

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

Change 907922 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/TemplateData@master] Add missing validation for empty parameter names

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

I went ahead and fixed the very obvious mistakes I could find via Global Search. Almost all of it was a copy from the English Navbar template.

The rest is most certainly from a time before we fixed a bug that actually generated parameters with no name from strange wikitext like {{{|}}}. It should be fine to leave most of these places untouched, especially the ones that are sandbox experiments or entirely unused.

Note that the regex search I used is limited and might miss a few places.

Change 907922 merged by jenkins-bot:

[mediawiki/extensions/TemplateData@master] Add missing validation for empty parameter names

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

Change 907920 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Fix template dialog when TemplateData contains empty parameter

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

matmarex assigned this task to thiemowmde.