Page MenuHomePhabricator

Pathological duplicate aliases could be displayed in sidebar menu
Closed, DuplicatePublicBUG REPORT

Description

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

  • create a template with parameters that have an alias
  • use the same alias for different template parameters (note: there is no check that prevents this)
  • include the template in a wikipage and enter some data to the alias (via wikitext mode)
  • open the visual editor to edit the template

What happens?:
All parameters that have the alias are checked in the sidebar, although the entered data is only displayed with the alias as title (see screenshot).

Screenshot from 2021-07-26 15-45-36.png (533×935 px, 46 KB)

What should have happened instead?:
Not sure. I gues the best way would be to prevent alias dublicates to happen?

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc:
Current master branch with active feature flag for the new sidebar.

Event Timeline

lilients_WMDE renamed this task from Dublicate aliases are displayed in sidebar menu to Duplicate aliases are displayed in sidebar menu.Jul 26 2021, 2:53 PM

+1 to "prevent alias duplicates", the TemplateData extension already (for better or worse) does blocking validation on its JSON. The editor should also prevent this condition.

Then, our sidebar code can assume that this will never happen.

I see that some related code has already been merged, but I'm not sure if there was intentional design work linked to the decisions? @ECohen_WMDE

We need to consider three possibilities: * template invocation uses a canonical param name, * uses an aliased param name, or * edge case where a canonical name and alias are both in use.

My suggestion is that we only ever show canonical parameter names in the sidebar. If an alias is in use, we resolve to the canonical name and hide the aliasing from our interface.

In the edge case where two different names are in use for one parameter, we should align with the actual behavior (if the template will take the last declared value, then that's what we show), and decide whether to throw away the unused value or silently keep it in the wikitext.

Not meant to avoid discussing this, but to document what I believe I know:

  • When a parameter does have a label, that's shown. In this case the fact that a parameter is actually used via an alias is entirely invisible.
  • The current behavior matches the original behavior: When a parameter doesn't have a label, the parameter name is shown as it is used in the article. That might be an alias. See the screenshot below (pink = old behavior, other_articles is an alias, the actual parameter name is other articles).
  • Sticking to the existing behavior was a decision we already made, as far as I remember.

Screenshot from 2021-08-13 09-15-21.png (302×473 px, 24 KB)

But this is not even what this ticket is about, as far as I understand. This ticket is about when there is more than one way to assign a value from a template invocation to a documented parameter. I agree this feels like a bug in the TemplateData validation. If it isn't, we should make sure VE behaves "sane" (i.e. close to how the wikitext parser behaves).

At the moment the behavior might appear "random". This is because duplicate aliases in the documentation are merged in a way that what appears last wins. But this order is possibly different from the template invocation where the winner is the one that appears last in the wikitext.

Note: I had a quick look at the current behavior and if there is a quick and easy way to make it appear less broken. I gave up. Every possible change feels awkward.

  • One thing we could do is to check the incoming TemplateData docs for duplicate aliases and clean that mess up. But it appears there is no way to know which of the duplicates is the bad one. And even if, doing so would tinker with the original docs, which is something we said we shouldn't do.
  • Another way to resolve the situation is to actually duplicate the parameter in the template invocation. The same value would then show up twice in the dialog, with both possible parameter names. This hopefully matches the template's behavior (or not if the duplicate alias is nothing but a mistake in the documentation). It gives the user a chance to see the error (or not because it's not obvious at all what causes the duplication). The main benefit of this approach is that it gives the user a chance to actually fix the error by removing one of the duplicates. The remaining parameter will be stored with it's primary name. The bad alias is gone. The main disadvantage of this approach is that it will cause dirty diffs in case the user tried to edit something else, but didn't care about the bad parameter.
  • Another way forward is to forget about both aliases, and present the bad alias as if it is an undocumented parameter. The main benefit of this approach is that we can pretty much guarantee that no dirty diffs can happen. Except the user starts to add the two documented parameters. While this will result in a clean diff (it reflects exactly what the user did in the dialog), it actually makes the mess in the wikitext worse (now there are up to 3 conflicting parameters, not only 1).
  • I think the best idea is to straight up deny editing all invocations of this template as long as the community didn't fix the bad TemplateData. It might be possible to limit this error messages to invocations that actually use the bad alias.
awight renamed this task from Duplicate aliases are displayed in sidebar menu to Pathological duplicate aliases could be displayed in sidebar menu.Sep 6 2021, 12:24 PM