Page MenuHomePhabricator

Message cite_error_ref_too_many_keys is confusing
Closed, ResolvedPublic

Event Timeline

Change 800611 had a related patch set uploaded (by A2093064; author: A2093064):

[mediawiki/extensions/Cite@master] Correct message too_many_keys and add conflict_follow

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

Suggestion is:

  • Brush up the patch that's adding the message for the follow case so it's mergable
  • Figure out if there are things to change for Parsoid
  • Create a ticket for the Cite-Extends part. We should have a custom error message there

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

[mediawiki/services/parsoid@master] Give a different error from too_many_keys when 'follow' attribute conflicts

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

I tried to understand better what the message cite_error_ref_too_many_keys even means. What's a "key"?

The message exists ever since the extension was created in 2005. Originally the user-facing text was "Invalid call; too many keys specified". This error was triggered literally when the <ref> tag had more than a single argument, no matter what these arguments are. It's important to note that the argument name name=… is nowhere to be found in this version of the code. That's because this version was means to be used like this: <ref foo> and <ref bar> where "foo" and "bar" really are the keys, but <ref foo bar> is forbidden.

That's why the message is worded like this.

3 days later the name=… argument appears for the first time. The check is expanded to fail for any argument that's not named name=…. Unfortunately this is not what the message says. Instead it now says "Invalid call; invalid keys, e.g. too many or wrong key specified". That's technically not wrong, but heavily confusing. "Key" still means "argument". "Wrong key" means "the only allowed argument is name=…". "To many" means "no other argument is allowed".

In other words: What the message really means at this point is "invalid argument". That's it.

In 2008 the group=… feature was added, and more arguments the following years. The message still means the same: that an unsupported argument was found.

When follow=… was added in 2010 the old message was used for a new special case: Sure, follow=… and name=… together are "to many" arguments. But each is valid. While it wasn't entirely wrong to use the old message it added a lot to the confusion.

The last time the message changed was in 2006, from "invalid keys, e.g. too many or wrong key specified" to "invalid names, e.g. too many". That was mostly a mistake that made the message just more confusing. Now it talks about names as if it's possible to have multiple names (which was true for 3 days in 2005!), but the (still correct!) hint that the error is about a "wrong argument" was removed.

Conclusion:

  • cite_error_ref_too_many_keys always meant "invalid argument", should be changed to say that more clearly, and only used in exactly this situation.
  • The fact that it's used for extends=… when Cite-Extends is disabled is actually correct. That's an unsupported argument then.

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

[mediawiki/extensions/Cite@master] Fix confusing wording of "invalid parameter in <ref>" message

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

Change 980461 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Fix confusing wording of "invalid parameter in <ref>" message

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

Change 800611 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Give a different error from too_many_keys when 'follow' attribute conflicts

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

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

[mediawiki/services/parsoid@master] Update legacy parser test with new error message from Cite

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

Change 983447 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Update legacy parser test with new error message from Cite

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

Change 983902 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.19.0-a9

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

Change 983902 merged by jenkins-bot:

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.19.0-a9

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

Change 980014 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Give a different error from too_many_keys when 'follow' attribute conflicts

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

Change 988723 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.19.0-a12

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

Change 988723 merged by jenkins-bot:

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.19.0-a12

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

Tobi_WMDE_SW subscribed.

remaining todo: UX & Engineering will assess the error messages together.