Follow-up for #3202664: CKEditor 5's UI language should match Drupal's interface translation to provide API for contrib projects to load CKEditor translations. This requires identifying which translation files exist, an associating them to the related library. Once the translation file is associated to the correct library, CKEditor 5 module is able to load the correct translation files based on the current UI language.
CKEditor 5 plugins needs to be compiled with the DLL build to be able to work with Drupal. This build automatically adds translations to the translations/ subdirectory. This is predicable because the original .po files used in the translation process are in a lang/translations/ sub-directory by convention.
The expected directory structure of a DLL build is the following:
ckeditor-plugin-name/
- translations/
- ckeditor-plugin-name.js
The only thing needed from contrib author is to declare that a plugin has translations and it can be done by adding a dependency to the core/ckeditor5.translations library in their plugin library declaration. Then Drupal will automatically find and load translations files.
myDrupalCkeditorPlugin:
js:
assets/js/ckeditor5/ckeditor-plugin-name/ckeditor-plugin-name.js: { minified: true, preprocess: false }
dependencies:
- core/ckeditor5
- core/ckeditor5.translations
With this declaration will automatically look for translations in the assets/js/ckeditor5/ckeditor-plugin-name/translations/* folder.
| Comment | File | Size | Author |
|---|---|---|---|
| #44 | core-ckeditor-trad-api-3228464-44-d9.patch | 1.58 MB | nod_ |
| #39 | core-ckeditor-trad-api-3228464-39-d9.patch | 1.64 MB | nod_ |
| #38 | core-ckeditor-trad-api-3228464-38-d9.patch | 1.64 MB | nod_ |
| #30 | core-ckeditor-trad-api-3228464-29.patch | 2 MB | nod_ |
| #28 | core-ckeditor-trad-api-3228464-28.patch | 1.64 MB | nod_ |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !1769
Comments
Comment #3
wim leersJust discussed with @Reinmar; there is no standardization around CKEditor 5 plugin development, and therefore also no standardization around directory structures either, including for example translations.
They actually just started looking into that!
Comment #4
wim leersWe should wait adding this until they've standardized it.
Comment #5
wim leersComment #7
wim leersPer @Reinmar: Recently https://github.com/ckeditor/ckeditor5-package-generator got explicit support for translations, so this should work fine now.
See https://ckeditor.com/docs/ckeditor5/latest/framework/guides/plugins/pack... + https://ckeditor.com/docs/ckeditor5/latest/framework/guides/plugins/pack...
We should verify this ASAP, so we can either confirm this is working and finish this issue, or report what is not working.
Assigning this to @bnjmnm because he built https://git.drupalcode.org/project/ckeditor5_dev/-/tree/1.0.x/ckeditor5_..., and I think that makes him the best positioned person to take a critical look at this 🤓
@bnjmnm, @Reinmar told me you should reach out to him prior to starting on this, because he thinks the docs at https://ckeditor.com/docs/ckeditor5/latest/framework/guides/plugins/pack... are not complete enough for you to get a good start on this!
Comment #8
wim leersComment #9
lauriiiI'm not convinced that the
ckeditor5-package-generatorwould help with solving this problem. It seems like the tooling is helping plugins to load translations from Transifex. However, what we need is an ability for contrib modules to load pre-built translation files.At the moment, we generate libraries for translations automatically in
ckeditor5_library_info_alter. These libraries are then loaded dynamically depending on the current UI language in\Drupal\ckeditor5\Plugin\Editor\CKEditor5::getLibraries. We need an API that provides equivalent functionality to contrib.I think the information that is needed is the directory where the translation files are located, and based on that CKEditor 5 module could handle generating libraries and then loading them later when needed.
Comment #10
lauriiiI think this is something that will have to be done on our side, unless CKEditor provides a new API that handles the discovery and loading of translations.
Comment #11
lauriiiI guess there's a second part which could potentially be relevant to contrib projects which is that we have a script in
core/scripts/js/assets.jsthat collects and concatenates translations from different plugins.Since it's discouraged for contrib projects to include 3rd party libraries, they will likely follow approach where they either use libraries system, or provide an intermediate package in Github that can be loaded using composer.
For either of these use cases it seems unlikely that the script would be useful. With the libraries system approach, they wouldn't want require users to run the script and with the composer approach they would likely have one repository per plugin.
I guess what is needed is documentation on what is the expected structure so that module authors can evaluate if the plugin they are integrating with is compatible with that. If the plugin isn't compatible, they will either have to provide integration themself, or run a script in the Github repository that makes the translation files compatible.
Comment #12
nod_can't be a feature request if it's a stable blocker no?
Comment #14
wim leers#12++
Comment #15
nod_All right so what changed:
Before
After
The vendor-update command is updated to do this now.
core/ckeditor5.translationslibrary needs to be added. It is done for the core libraries where relevant. If you declare the dependency and no translation files exists you will end up with a 404 script when the page loads.Comment #16
nod_One issue is that on load the language is not applied (for basic_html) but when I change to full_html the language is applied properly, and going back to basic_html it's back to english. Will need to find what happens there.
One thing not done is mapping drupal langcode to ckeditor langcode. That might cause a few issues, there is a todo in there.
Comment #17
nod_Issue is in the settings merging. When the language plugin is enabled it overwrites the language setting and the editor is not switched to the appropriate language.
Comment #18
lauriiiAdding related issue to #17
Comment #19
nod_Comment #20
nod_Sent a patch for #3250191: Translation of toolbar button tooltips not working when text part language plugin is enabled, that part of the patch is removed from this MR.
Comment #21
wim leers#15 makes a ton of sense — thanks so much for that explanation! 😊
Also see #3250191-24: Translation of toolbar button tooltips not working when text part language plugin is enabled — that is IMHO a hair removed from RTBC.
Wow GitLab fails spectacularly when looking at this merge request… :O I spent more time trying to scroll to the relevant piece and trying to post a comment at all than actually reviewing…
Review posted on the MR.
Comment #22
nod_Comment #23
wim leersI just found out this is implicitly blocked on #3262160: Simplify code in assets.js, remove mix of await and promise code — making that explicit now.
Comment #24
nod_Comment #25
wim leersI think this is RTBC once we also have green patches for
9.3.xand9.4.x🚀🥳 And thanks to the existing test coverage plus the bug that was fixed a few days ago with test coverage (#3250191: Translation of toolbar button tooltips not working when text part language plugin is enabled), we can be very confident that this approach is actually working. 👍The changed strategy in CKE5 asset loading makes sense. It is conceptually super simple, simpler than before, which is a good quality 😄 But on the flip side, there's more individual assets to load, which makes me want to mark #3224261: [PP-1] Front-end performance: concatenate CKEditor 5 JS assets for every unique configuration a stable blocker. That's an absolutely fine trade-off IMHO.
P.S.: reviewing this on GitLab is now officially impossible. Had to clone it locally and generate diffs for specific files. 🙃 (f.e.
git d 302b310392849b95c9aee41542f272de79722a5e HEAD -- core/scripts/js/assets.js)Comment #26
nod_not quite :)
When aggregation is enabled it's the same as before: 1 file containing all the translations, separate from the other aggregates. So no overhead when aggregation is ON.
Comment #27
wim leers#26: yes, I realize that — I did mean in the scenario when aggregation is not on :)
Can we get 9.3 & 9.4 patches? :)
Comment #28
nod_9.4 patch, waiting on the other backport for 9.3
Comment #29
wim leers#3262160: Simplify code in assets.js, remove mix of await and promise code was backported to 9.3! I think that means #28 can be tested against 9.3 as well?
In any case, this means that this issue now is committable to 2 out of 3 branches already, so moving ahead to RTBC :)
Very nice work here! 👏
Comment #30
nod_9.3 version this is still ckeditor 31.0.0 in this branch so a bit less translations available.
no idea why the file size is so different though
Comment #31
lauriiiPosted few comments on the MR but this looks really close to being ready for me 👍
Comment #32
nod_Added some documentation and fixed a few things to make the rendered page better (currently it's https://api.drupal.org/api/drupal/core%21modules%21ckeditor5%21ckeditor5...) the @see can't be multiline, added some headers to the code blocks to make it more explicit what/where things are happening.
Comment #33
wim leersComment #34
lauriiiI think it would be good to write change record for the new API as well.
Comment #35
wim leersGot confirmation from the CKEditor 5 team:
🚀
Comment #36
nod_Added a CR https://www.drupal.org/node/3264983
Comment #37
wim leersChange record looks great 😄
Comment #38
nod_D9 version
Comment #39
nod_fix bad reroll
Comment #42
lauriiiCommitted to 10.0.x and 9.4.x. We probably want to backport this to 9.3.x so that contrib can start using this. We still need a separate patch for that.
Comment #44
nod_9.3 version of the patch. It's still using ckeditor 31.0.0 and not the 31.1.0 we have in d10/d9.4
Comment #45
hooroomooreviewed 9.3 patch and tested locally that expected translation files load with es and af locale codes. LGTM :)
Comment #46
wim leers#44: Woah!! I think it is a mistake/oversight that #3258250: Update to CKEditor5 v31.1.0 never was cherry-picked to 9.3?!
Comment #47
lauriiiCommitted 33f8ef2 and pushed to 9.3.x. Thanks!