Page MenuHomePhabricator

Validations for gadget definitions
Open, Needs TriagePublic

Description

Validation warnings to show:

  1. Gadget with type=styles having non-CSS files
  2. Packaged gadgets not having at least one JS file (after T198758)
  3. Non-packaged gadgets having JSON files (after T198758)
  4. Style-only gadgets having peers
  5. Peer gadgets not being styles-only gadgets
  6. Scripts/styles/datas not being of JS/CSS/JSON contentmodel respectively, or not existing altogether
  7. Invalid page actions specified
  8. Non-existent namespace numbers specified (after https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Gadgets/+/624517/) (done)
  9. Invalid targets specified
  10. Invalid content models (specified as load condition)
  11. Default gadget requiring ES6

Currently all of these validations do take place at some point, but no warnings are shown to the user. The problematic parts in gadget definitions are silently ignored.

Approaches

  1. Expand GadgetDefinitionValidator (for gadget definition namespace only) – Pre-save hook that blocks the save in case of issues. The existence/contentmodel of the resource files could change, so it may not be a good idea to validate those here.
  2. Show warnings in Special:Gadgets – These can cover all kinds of validations.

Event Timeline

SD0001 renamed this task from Show gadget definition validation warnings in Special:Gadgets to Validations for gadget definitions.Jan 4 2022, 8:53 PM
SD0001 updated the task description. (Show Details)

Adjusted description to focus on the problem, and also consider other approaches.

A warning or error during save makes sense for the things which are local to the definition (ie. all but 5/6). A warning on Special:Gadgets makes sense for every type, since even correct definitions might become incorrect over time as the validation logic changes (plus it's probably needed for the MW page repo anyway, to avoid an incorrect gadget definition blocking changes to all other gadgets).

On Invalid page actions specified: The set of possible actions is unlimited.

  • Once there has been a parsermigration-edit.
  • They do no harm.
    • If they are mentioned, but nobody ever is running a URL with that action they have no effect.
    • If somebody misspelled edti the gadget will not start and this will be discovered quite soon.
  • raw is a valid action but it is hard to imagine a meaningful gadget operation.

I think for 7 and 8, non-existent namespace and invalid page action, an error should prevent save when editing the gadget definition in the first place, but allows on subsequent attempt if the user confirms indeed that's what they want. This would allow actions=never and namespaces=-999 as some kind of disablement mechanism and are harmless.

Then apply same treatment to the other issues (1-6 above) but additionally also show a persistent warning on Special:Gadgets or non obtrusive link to a separate error page with detailed info.

It is hard to find a definite list of action= since those may depend on extensions and vary over time.

  • action=markpatrolledExtension business, not activated in every wiki project
  • action=deletetrackbackvalid but not active in WMF
  • action=dublincorevalid but not active in WMF
  • action=creativecommonsvalid but not active in WMF
  • action=creditsvalid but not active in WMF
  • action=parsermigration-edithas been valid but pointless today

I might want to set namespace numbers not defined in this wiki right now (100, 101 etc. are configured per wiki), e.g. 2300 in advance for future gadget definition namespace.

Your messages will be more confusing than helpful here. Unknown actions and namespaces do no harm, they are never reached, and it is up to experienced interface administrators to clear useless conditions.

However, valid numbers and actions are more risky than invalid ones: Gadgets would start on valid but undesired context. This is not detectable by standard rules. You need human brain.

Is Special:Gadgets intended for end-users or gadget developers? I always assumed it was the former and the Gadgets-definition page was for the latter.

Assuming that, my preference would be to show validation errors and/or warnings on the Gadgets definitions page, as verbosely as possible (though not necessarily blocking page save) to help gadget developers. And Special:Gadgets would only show critical errors or no errors at all.

Is Special:Gadgets intended for end-users or gadget developers? I always assumed it was the former and the Gadgets-definition page was for the latter.

I think both of them are for developers. The end-user version is in Special:Preferences.

Assuming that, my preference would be to show validation errors and/or warnings on the Gadgets definitions page, as verbosely as possible (though not necessarily blocking page save) to help gadget developers. And Special:Gadgets would only show critical errors or no errors at all.

Valid gadgets can become invalid as the extension code changes, as dependencies change, as the component scripts/styles/data-files change etc. Showing that on Gadgets-definition when it's edited isn't really helpful. Showing it on Gadgets-definition when it's not edited is still not terribly helpful (people don't visit that page much other than for editing it, I think) and also somewhat hard.

Is Special:Gadgets intended for end-users or gadget developers? I always assumed it was the former and the Gadgets-definition page was for the latter.

I think both of them are for developers. The end-user version is in Special:Preferences.

Ack. Going with that then, showing errors/warnings in either or both seems fine.

Assuming that, my preference would be to show validation errors and/or warnings on the Gadgets definitions page, as verbosely as possible (though not necessarily blocking page save) to help gadget developers. And Special:Gadgets would only show critical errors or no errors at all.

Valid gadgets can become invalid as the extension code changes, as dependencies change, as the component scripts/styles/data-files change etc. Showing that on Gadgets-definition when it's edited isn't really helpful. Showing it on Gadgets-definition when it's not edited is still not terribly helpful (people don't visit that page much other than for editing it, I think) and also somewhat hard.

I don't know why it would be hard, just drop some HTML above the object table output. These pages are so cheap to render that we don't need to worry about the parser cache so logged-in users will always see up to date warnings.

I don't know why it would be hard, just drop some HTML above the object table output. These pages are so cheap to render that we don't need to worry about the parser cache so logged-in users will always see up to date warnings.

Right, but ideally you'd show the warnings in-context, not on the top of the page. And warnings on Special:Gadgets will be needed anyway, because in namespace repo mode there is no other place to put them (or rather, putting them on the gadget definition pages is useful, but we'd want a single-page overview where it's easy to check whether anything is wrong).

Warnings on Special:Gadgets are confusing and frustrating for regular users since that audience is not able to take action. If these messages are displayed over years nobody will take care about any warning.

On Special:Gadgets the maximum I could think of is a consolidated message box in the footer area, that errors have been detected and local or maintaining global interface admins should be notified now. They will be happy to receive a stream of messages from smaller wikis.

On definitions page I already made the suggestion above to develop an interactive form, which will read the current content, parsing every line, showing warnings if problem. Then offering for each entry tic boxes, list of options for closed or open set of items, clickable links for transcluded resource pages, page selection of possible resource pages within namespace to be added, removal of resource pages. Finally, save the entire page, hopefully in valid syntax now.

Why would regular users care about Special:Gadgets at all?

"Don't show warnings, otherwise people won't fix the problems" seems like rather suspect argumentation to me.

Users in projects without interface admins might want to export, check, control their offered gadget preferences, but they are not able to edit them. They will need global help.

Therefore I proposed: “consolidated message box in the footer area, that errors have been detected and local or maintaining global interface admins should be notified now”

Details are not meaningful in this place.

If someone creates a Definitions Editor and that works correctly and is offered to all experts, errors will occur rarely.

Otherwise the definitions syntax is not very human friendly if several dozens of gadgets with sophisticated conditions are listed.

Beside default usage of Definitions Editor a source access to definitions page is required if severe syntax errors are to be remedied.

Manpower should be directed into JavaScript to develop an interactive Definitions Editor rather than into PHP to issue sophisticated warnings on bad definitions syntax. Errors might be detected, if any, or just ignored. Details will be presented by interactive form. If a required resource page has been moved or deleted, who shall investigate this? Avoid double work. As far as I learned from the PHP code there is no deep analysis of precise definitions syntax yet.

  • If somebody misspelled edti the gadget will not start and this will be discovered quite soon.

I'd say this assumption is incorrect as it is not my experience at all. See T320650 or T320573. Many wikis have a small user base or do not have interface admins.
(I came here as I was about to file a ticket to request validation of syntax of entries on MediaWiki:Gadgets-definition.)

If you refer to:

  • edittools-magnitudo[ResourceLoader]|actions=edit|edittools-magnitudo.js

Well, this is a fundamental syntax error at least on first glance.

In this case the gadget is always started, but not limited to actions=edit.

Here the entire statement actions= is currently ignored, but my remark was heading for a case where actions= has been processed but the action code edti or conjure is not within limited vocabulary.

Therefore this is not 7. Invalid page actions but no page action at all.

A white list of permitted page actions would need maintenance every time some further action has been invnted by any extension.

Maybe it would be more useful to spit back interpretation of what was entered. You know like now when enable ES6 it says that on the special page ("This gadget is only supported on ES6-compliant browsers") and other thing like that. So like that, but show even a dump of options on a preview of changes or something. I guess an editor would solve that too (like for TemplateData).

Change 968727 had a related patch set uploaded (by SD0001; author: SD0001):

[mediawiki/extensions/Gadgets@master] Show gadget definition validation warnings in Special:Gadgets

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

SD0001 updated the task description. (Show Details)

Change 968727 merged by jenkins-bot:

[mediawiki/extensions/Gadgets@master] Show gadget definition validation warnings in Special:Gadgets

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

Not fully resolved I think. The merged patch shows warnings on Special:Gadgets. For the JSON definitions format, we can additionally surface them on preview or before save.

Change 983512 had a related patch set uploaded (by SD0001; author: SD0001):

[mediawiki/extensions/Gadgets@master] Expand pre-save validations for JSON definition pages

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

Change 983519 had a related patch set uploaded (by SD0001; author: SD0001):

[mediawiki/extensions/Gadgets@master] Add validations for skins and rights

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

Could we validate against gadgets that use module dependencies that are marked as "deprecated" (assuming these are non-blocking) ? I think that would be really helpful.

Change 983512 merged by jenkins-bot:

[mediawiki/extensions/Gadgets@master] Expand pre-save validations for JSON definition pages

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

Change 989242 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/extensions/Gadgets@master] Avoid global RequestContext in GadgetRep::validationWarnings()

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

Change 989242 merged by jenkins-bot:

[mediawiki/extensions/Gadgets@master] Avoid global RequestContext in GadgetRep::validationWarnings()

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

Change 983519 merged by jenkins-bot:

[mediawiki/extensions/Gadgets@master] Add validations for skins and rights

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