Page MenuHomePhabricator

Extend message checker framework to support errors that prevent saving
Closed, ResolvedPublic

Assigned To
Authored By
Nikerabbit
Sep 17 2018, 5:32 PM
Referenced Files
F29774857: image.png
Jul 16 2019, 7:53 AM
F28594941: flagY30-ltr.svg
Apr 8 2019, 12:11 PM
F28594940: errorR30.svg
Apr 8 2019, 12:11 PM
F28594943: message-icons.png
Apr 8 2019, 12:11 PM
F28594969: message-expand.png
Apr 8 2019, 12:11 PM
F28594942: flagY30-rtl.svg
Apr 8 2019, 12:11 PM
F28593427: 2019-04-08_00-04-T204568.png
Apr 7 2019, 6:55 PM
F28593425: 2019-04-07_23-54-T204568.png
Apr 7 2019, 6:55 PM
Tokens
"Love" token, awarded by Pintoch.

Description

The issue of build breaking translations has come up frequently (one recent example).

For some message groups, the warnings about unused or extra parameters should be promoted to errors, that will prevent saving those translations in the first place, as might happen by careless translators.

Workarounds in use:

  • Project maintainers manually remove bad translations until they are fixed, after each export (given exports twice a week, a lot of wasted time)
  • Translatewiki.net admins put in English translation in place and mark it as outdated (may break fallback chains)

Suggested solution:

  • Refactor MessageChecker classes to introduce separation between warnings and errors.
  • Use one of the pre-save hooks to run the error checks and fail the save if they fail.
  • Show error messages in the UI
  • Translation admins should be able to override (to not break message renames etc. with existing issues)

It would be good time to also look at the insertable classes, and see if those can be combined with message checkers to reduce duplication.

Related Objects

StatusSubtypeAssignedTask
ResolvedNone
ResolvedNikerabbit
Resolvedabi_
Resolvedabi_
ResolvedNikerabbit
Declinedabi_
Resolvedabi_
Resolvedabi_
Resolvedabi_
Resolvedabi_
Resolvedabi_
Resolvedabi_
ResolvedCrutishnyk
Resolvedabi_
Resolvedabi_
Resolvedabi_
ResolvedCrutishnyk
Resolvedabi_
Resolvedabi_
Resolvedabi_
Resolvedabi_
ResolvedKizule
Resolvedabi_
ResolvedCrutishnyk
Resolvedabi_
Resolvedabi_
Resolvedabi_
Resolvedabi_
Resolvedabi_
Resolvedabi_
OpenNone
Resolvedabi_
OpenNone
Resolvedabi_
ResolvedBUG REPORTNikerabbit
Resolvedabi_
Resolvedabi_
OpenFeatureNone
ResolvedFeatureabi_
OpenNone
Resolvedabi_
Resolvedabi_
ResolvedSn1per

Event Timeline

Pginer-WMF triaged this task as Medium priority.Nov 13 2018, 8:48 AM

We have this problem in https://dissem.in/ . This project is set up on Translatewiki, the code is hosted on GitHub and uses Travis for CI. We use Django's localization system which is based on gettext. We compile messages in the CI to check that they are valid. Sometimes translators add incorrect translations (such as translations not reusing the same variables as the msgid, or in a different format). This breaks our build as any incorrect translation will stop the entire compilation process. It is not clear if and how it would be possible to configure the translation compilation process to ignore invalid messages.

(by the way we are very happy that we have set up Translatewiki, it's great to see the website translated in so many languages, thanks to @Nemo_bis )

Nikerabbit raised the priority of this task from Medium to High.Mar 18 2019, 3:45 PM
Nikerabbit added a project: User-abi_.

Change 500243 had a related patch set uploaded (by Abijeet Patro; owner: Abijeet Patro):
[mediawiki/extensions/Translate@master] Adds a new MessageValidator framework to improve customizability

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

@Pginer-WMF / @Nikerabbit - Need inputs from you for the UI changes on this task,

UI Modifications

Following summarizes the changes to the UI.

  1. Errors refer to notices that have to be fixed in order for the translation to be saved. Errors are represented in red on the UI, and always appear above the warnings.
  2. When the user makes an error (forgetting a closing brace etc), the save translation button is disabled with a delay of 0.5 to 2.5 secs. This is because the validation is done on the server side.
  3. Since it might be possible for the user to click on the save button in the meantime, or use the shortcut Ctrl + Enter to save the translation, a validation is also done on translation save API.
  4. Upon clicking the save translation button, the user is taken to the next translation to make it easy to quickly switch between translations.
  5. After saving if the validation fails, the user can see the message highlighted in yellow, similar to what happens now incase of a save failure.
  6. Upon clicking the message, the user can then see all the notices, with the more notices tab expanded.

Few other tweaks were made,

  1. background-position-x for the flag was modified as it was appearing slightly off the center.
  2. Colors for the errors were taken from here - https://design.wikimedia.org/style-guide/visual-style_colors.html

Doubts

  1. Currently for warnings we use a flag as icon, any recommendations for errors? I think we can use the general error icon (the X on the red background).
  2. We want to allow translation admins to be able to save even if there is an error. There are two considerations here,
    1. The validation that occurs when the translation text changes. In this case, the translation admin will see the warnings but the button will not be disabled even if there is an error.
    2. If the admin saves the translation before the textchange validation can occur, the admin will not see the errors at all, and the validation will be saved. I could try to disable the save button while the server validation is occurring, and wait for the response but that might be irritating for the user and error prone, since I cannot blindly enable the button after the call, but rather set it back to its previous state.
  3. There is currently a "yellow" button to show more notices if more than 1 notice exists. This will need to be updated to make it something more color neutral.

Screenshots

Here are a few screenshots,

  1. When there is a validation error from the server upon clicking save,

2019-04-08_00-04-T204568.png (713×1 px, 59 KB)

  1. Validation error shown on text change,

2019-04-07_23-55-T204568.png (667×1 px, 54 KB)

  1. Collapsed more tab

2019-04-07_23-54-T204568.png (659×1 px, 51 KB)

@Pginer-WMF / @Nikerabbit - Need inputs from you for the UI changes on this task,

Thanks for the very detailed update, @abi_ !

  1. Currently for warnings we use a flag as icon, any recommendations for errors? I think we can use the general error icon (the X on the red background).

As part of the design style guide efforts, icons have been updated since these UIs were designed. Base, on the icons from the icon repo we can use "flag" and "error" icons.

I created color versions (using colors from the palette) for both:



I made a quick mockup to show how those would look like:

message-icons.png (713×1 px, 77 KB)

  1. We want to allow translation admins to be able to save even if there is an error. There are two considerations here,
    1. The validation that occurs when the translation text changes. In this case, the translation admin will see the warnings but the button will not be disabled even if there is an error.
    2. If the admin saves the translation before the textchange validation can occur, the admin will not see the errors at all, and the validation will be saved. I could try to disable the save button while the server validation is occurring, and wait for the response but that might be irritating for the user and error prone, since I cannot blindly enable the button after the call, but rather set it back to its previous state.

If checkings take time it is ok to let the user to move forward, but we also want the user to be aware of possible issues when those are surfaced. Once the admin saves the message, the message becomes compact and the next is expanded. It would be great for the errors to be shown as tags in the compact message. Is that already the case?
A red tag in the message above should be visible enough. If more attention is needed we can consider increasing the prominence by either:

  • Highlight the background of the affected element in light red for half a second once the validation has been completed (only if the message got validated when already being compacted).
  • We can also consider a bubble notification that indicates that "A message was saved with errors", if we expect the affected messages to easily go out of the viewport.
  1. There is currently a "yellow" button to show more notices if more than 1 notice exists. This will need to be updated to make it something more color neutral.

We can adapt the button color to use #FFBBBB when it is shown next to an error message to keep the colors consistent (and keep the current color when it appears next to the yellow ones):

message-expand.png (659×1 px, 67 KB)

@Pginer-WMF - Thank you for your response.

  1. I'll use the icons that you've linked to. I'll update the flag icon as well.
  2. I will review and decide which one to use.
  3. That can be done, but will require checks at various places. We will have to check and update the color of the button every time a notice is added / removed and when the section is expanded / collapsed. To avoid having to add that complexity, I wanted to use something more color neutral. I will discuss this once with Niklas and let you know if I need further inputs.

Test cases

Following are the manual test cases that were run,

  • Tested to ensure that all old suggesters and checkers still work fine. This was done using the EtherpadLite message group.
    • Suggestions are appearing.
    • Warnings appear if certain keywords are not used.
    • User is allowed to save even if warnings are present.
  • Updated MathJax message group to use the new validators and tested,
    • Insertables are appearing.
    • If enforce: true is set, errors appear as per configuration.
    • If enforce is set to false or not present, warnings are shown instead of errors.
    • A normal translator is allowed to save when warnings are present.
    • A normal translator is not allowed to save when errors are present.
    • Ensure validations are not performed for message documentation.
  • Updated MathJax to use new Insertable configuration format and tested,
    • Insertables are appearing.
    • If insertables are skipped no warnings are appearing.
  • Added a custom validator and ensured that it is working.
  • Activated the Timeless theme to ensure the updated functionality was working there and that the UI was OK.
  • Tested message renames to ensure that it was working properly.
    • Renamed a few keys on the local message group projects (MathJax and EtherpadLite)
    • Ran processMessageChanges.php.
    • Opened Special:ManageMessageGroups to see the changes reflected.
    • Then used Special:ReplaceText to rename the pages.
    • On Special:ManageMessageGroups, ignored and submitted messages for import
  • If translation has an error, then message is not saved in case the user is a translator.
  • If translation has an error, message is still saved if the user is an translation admin.
  • Tested pre-provided validators,
    • BraceBalanceValidator
    • InsertableRubyVariableValidator
    • MediaWikiMiscValidator
    • MediaWikiPluralValidator
  • Tested validations for WikiPageMessageGroup
  • Ensured that validators and checkers work well together. Configured a message group to have both and tested.

During testing, found and fixed T220789: Invalid "more warnings" label shown during message translation

A summary of the new functionalities is documented here. We've also introduced namespaces in the code in Translate extension.

Note that some of the Gerrit links in the above document will not function at this moment, but should start working once the changes have been merged to the master branch.

While the above document covers the overall functionality, following documents will have to be updated as well,

  1. New document to be created regarding Validators - https://www.mediawiki.org/wiki/Help:Extension:Translate/Validators
  2. Extension:Translate/Group configuration example

Future changes

During the development of this task, a few more changes were identified which can be made in the future. I'll discuss with Niklas and create the necessary tasks.

  1. We should review the InsertablesSuggester and see if these can be moved under a namespace as well. In addition, we can introduce a Factory method similar to what's done for Validators. This might again need an update
  2. Currently if an admin saves a translation that has errors. They are not given any warning that the messages that they have saved had errors. A solution to this was discussed with Pau but have decided to not implement this right now, as finding the right hook to do this was becoming cumbersome.
  3. Some of the icons used in the translate extension can be updated. The CSS for all icons can be standardized, but we will need icons with the same height / width, and with the same amount of space / padding.
  4. MessageCheckers should be deprecated. I think we should discuss with the community and identify projects on Translatewiki.NET that can start using MessageValidators and then subsequently once we decide changes on point 1, we can start having other projects use MessageValidators and eventually remove the MessageCheckers.
  5. We should perhaps review how external dependencies (like Parser ) could be injected to the validators.

Change 500243 merged by jenkins-bot:
[mediawiki/extensions/Translate@master] Add new MessageValidator framework to improve translation validation

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

Awaiting deployment on Translatewiki.net

This has been deployed on Translatewiki.net.

We recently added a new project to Translatewiki.net - Wikimedia Grants review (T74291). This project is using the new Validator framework for insertables and showing warnings to translators. See patch. See screenshot below,

image.png (755×1 px, 82 KB)

Any other issues will be opened as a separate task / bug.

abi_ moved this task from QA to Done on the User-abi_ board.

Change 535511 had a related patch set uploaded (by Abijeet Patro; owner: Abijeet Patro):
[mediawiki/extensions/Translate@master] Add new MetaYaml schema format for INSERTABLES

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

Change 535511 merged by jenkins-bot:
[mediawiki/extensions/Translate@master] Add new MetaYaml schema format for INSERTABLES

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