Page MenuHomePhabricator

Prevent (or list) musical notation with syntax errors
Open, Needs TriagePublic

Description

Problem: Wikibase lets users write free text as a value for the "Musical Notation" data type. When a syntax error is introduced, the value is accepted and saved and the following message appears.

invalid-lilypond.png (326×948 px, 14 KB)

Syntax errors should be prevented or, if that's not possible/convenient, listed on some page to easily access and fix them.

From Task Time 11.02.2026
Task has three parts

  1. Noting unexpected UX of values being accepted despite syntax errors
  2. Feature request to prevent these errors
  3. Feature request to list syntax errors on a page

Acceptance Criteria

  • Investigate this issue in light of the recent changes made in the MEX project (T394621)
  • Confirm if this behavior is unique to lilypond or not (behavior repeats in other datatypes)
  • Summarize potential next steps

Timebox : 8 hours

Event Timeline

For comparison: the mathematical notation datatype does prevent saving invalid edits (though the text input stays disabled, so it’s not a recoverable error because you can’t edit your input anymore to correct it, but that’s another issue):

image.png (164×876 px, 23 KB)

Had a quick look. Even on the desktop interface for musical notation, the expert is calling wbformatvalue the whole time that you are editing and already getting the error responses, so that is something we can do in wbui2025.

The current implementation in wbui2025 for score is the ScoreValueStrategy (extensions/Score/modules/wikibase.wbui2025/score.wbui2025.entityViewInit.js), which is just a direct subclass of the StringValueStrategy. In that case we are calling wbparsevalue every time the value changes, but not wbformatvalue. But if you look at what the editableTimeSnakValue.vue does, it watches for changes to the text input then hits wbformatvalue via the call to debouncedTriggerFormatAndParse.

It should be possible to create a custom editableScoreSnakValue.vue component in the Score extension, require it in score.wbui2025.entityViewInit.js and wire it into the ScoreValueStrategy by implementing getEditableSnakComponent() (which can return either a string or a reference to a Vue component - we'll return the Vue component). We can then implement a watcher on textvalue as we did in editableTimeSnakValue.vue to trigger the formatvalue call. Once wbformatvalue returns, we can parse the response to see if it's an error, and act accordingly.

To disable the save button, we need to change the definition of 'isIncomplete'. That could be as simple as introducing a haserrors field in the editSnak store (editStatementsStore.js), including haserrors in the evaluation of isIncomplete, and setting haserrors to true if we notice that the result of wbformatvalue includes errors.

There might also be a way to avoid copy-pasting code from editableStringSnakValue to editableScoreSnakValue. Instead of creating a new Vue component, we could add a few more functions to the ValueStrategy interface and make editableStringSnakValue smarter - it could ask the valueStrategy whether the value needs formatting (shouldFormatValueWhileEditing), and have a function for detecting errors (formatValueResponseContainsErrors). In that way we can avoid creating editableScoreSnakValuein the first place and simply implement a couple of new methods in the ScoreValueStrategy. Which also has the advantage that we could re-use it for other validations (e.g. mathematical notation).

The current implementation of mathematical notation avoids this problem because the Math extension validates the values by providing a 'validator-factory-callback' (extensions/Math/src/WikibaseHook.php). In Score (extensions/Score/includes/Hooks.php), this has helpfully been left out:

			'validator-factory-callback' => static function () {
				global $wgScoreMaxLength;
				// load validator builders
				$factory = WikibaseRepo::getDefaultValidatorBuilders();
				// initialize an array with string validators
				// returns an array of validators
				// that add basic string validation such as preventing empty strings
				$validators = $factory->buildStringValidators( $wgScoreMaxLength );
				// $validators[] = new ScoreValidator();
				// TODO: Take out the validation out of Score
				return $validators;
			},

From the version history it's not clear why this is the case or what the plan was here, but I could imagine that calling out to Lilypond is pretty expensive, and doing the validation on a lot of Score properties at save time would make restoring data from dumps prohibitively slow.

Change #1248420 had a related patch set uploaded (by Hasan Akgün (WMDE); author: Hasan Akgün (WMDE)):

[mediawiki/extensions/Wikibase@master] Prevent saving invalid musical notation

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

Change #1248423 had a related patch set uploaded (by Hasan Akgün (WMDE); author: Hasan Akgün (WMDE)):

[mediawiki/extensions/Score@master] Prevent saving invalid musical notation

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

I thought the intention behind this task was to prevent these edits server-side, not to make some of our frontends stop trying to save them. AFAICT, the linked patches still allow other API users to save invalid statements.

@Lucas_Werkmeister_WMDE Yep - that's true. I certainly interpreted this as a MEX task because it ended up in this sprint. What's your feeling about adding a ScoreValidator to the callback? Do you think that is practical / sensible from a resource utilisation point of view? And do you think it might be helpful to merge the patches into MEX (or split those off into a MEX ticket) so that we can move forward without ultimately resolving the Wikibase issue for now?

I think resource-wise, adding a validator for musical notation values should be fine, as it would only run when those values are added or changed, which should be pretty rare in the grand scheme of things.

Whether the validation should happen in MEX or for all edits sounds like a product question to me; but if we’re going to do it server side then IMHO we shouldn’t also do it in MEX, that’s just sending unnecessary extra code to the browser.

I agree with @ArthurTaylor because I also thought if it's in the sprint it's MEX, and I'm still on the same page. I think right now MEX itself solves its own problem without any server-side changes, hence our main focus is MEX I'm eager to go with current patches.

Task time notes:
Move to M5

@Alice.moutinho will look into the updated behaviour and create a design for errors that will appear when editors try to publish incorrect syntax for musical notation

This could possibly be a breaking API change

(Just for the record, since I already looked it up: the acceptance criterion “Invalid input can be saved” was originally added in T208489#4927651, presumably during a story time meeting.)

karapayneWMDE subscribed.

Additional notes: implementing the fix at the API level (per the ticket's original description) would be a breaking change and we would need to notify the community and wait. Once we have the API-level validation, the behaviour will be the same as it is for Math, which is that you get an error message when you click Save (but no indication that anything is wrong until you click save)