Third option for the CKEditor 5 "Language" button: `site_configured` (in addition to `un` and `all`)

Wow, thanks for the heads-up! I had no idea that this was happening! 😄

I managed to unblock @ifrik as she was leaving DDD2022! 🥳

Marking as Active because I think a patch will appear here shortly 🤓

Getting close! One more test to update :)

This new feature would also mean that having unit tests for this plugin becomes all the more important. Fortunately, #3229078: Unit tests for all @CKEditor5Plugin plugin classes is doing precisely that.

And now that I think about it: let's interpret [PP-1] as being postponed on that, because it'd actually be fine to land this before #3192734: Third option for the CKEditor 4 "Language" button: `enabled` (in addition to `un` and `all`).

Because, ironically, it is easier to write (thorough) tests for CKEditor 5 than for CKEditor 4, hence this might land sooner. 🥳 (Whichever of the two lands last will be responsible for adding the upgrade path from CKEditor 4.)

This looks almost perfect! Left two nits and one actual change request on the merge request.

I'm gonna put this on hold until after #3229078: Unit tests for all @CKEditor5Plugin plugin classes lands now, because after that lands, this MR should add one more test case to LanguagePluginTest's data provider 🤓 Until that lands, writing a unit test like that is painful. And the change I requested will help make writing that unit test simple :)

#3229078: Unit tests for all @CKEditor5Plugin plugin classes landed, so I think we can actually do this now! 🥳

We just need to update \Drupal\Tests\ckeditor5\Unit\LanguagePluginTest::providerGetDynamicPluginConfig() :)

Almost! 🥳

The release managers told us that no new features were allowed in CKEditor 5 compared to CKEditor 4 prior to shipping it in 10.0.

10.0 shipped. So now we can!

I've RTBC'd #3227354: Add support for ckeditor5-autoformat. I'd love to RTBC this as well!

Would you mind rerolling this one more time, @ifrik? 😊🙏

Fixed failed commands on #31
Attached patch against Drupal 10.1.x

@pooja saraah There is no interdiff: what changed? And: could you please just commit those changes to the merge request? 😊🙏

Did a detailed review — found a few small problems. Most importantly: the absence of RTL test coverage!

Still one last test failure 😅

And … AFAICT the feedback has not yet been addressed?

Marked #3375157: [upstream] CKEditor 5 language selector cannot be scrolled if "All NNN languages" option selected and #3375394: Add option to use website languages as the CKEditor5 drop-down languages as duplicates.

Given the wide interest in this (which is somewhat surprising given that the current functionality exactly matches that of CKEditor 4 in Drupal 8 & 9!), bumping to Major.

Updating title from CKEditor 4 to CKEditor 5.

Thank you for the credit. Apologies for not diving in and coding. I don't have the bandwidth to set up a test environment and a way of applying my code.

For rigorousness, would it be better if the test language were not one of the UN six?

Currently the test is add English and French, then test that choosing enabled produces English and French.

I respectfully suggest the following:

Add English as the site base language, French as the non-base UN language, and a language that is in the all list but not in the UN list, for example, Chinese (Traditional), as website languages.
Choose Enabled for CKEditor languages.

Then test:
- that English is in the CKEditor list (testing the base website language)
- that French is in the CKEditor list (testing a UN language which is a website language)
- that Chinese (Traditional) is in the CKEditor list (testing a non-UN language which is a website language)
- that Chinese (Simplified) is not in the CKEditor list (testing a UN language which is not a website language)
- that Portuguese is not in the CKEditor list (testing a non-UN language which is not a website language)

Doesn't have to be these specific languages, but does need to cover all four entries in the truth table plus the base language.

Note that I made major edits to the previous comment. If you receive comments via email, please check the current version.

In #3383582: Add option to select from Drupal's enabled languages to CKEditor "Language" button I added to the list of duplicates of this ticket, then discovered this one.

Looks like the MR here wants a rebase?

One thing on my mind is the language in the selector - "All 107 languages" doesn't include any custom languages configured on the site, so I feel a more accurate term is "Drupal predefined languages". "All" is a misdirecting term when it may not include any custom languages of the site itself, and there are many languages not present in LanguageManager::getStandardLanguageList().

In the WIP-until-just-now MR at https://git.drupalcode.org/project/drupal/-/merge_requests/4655 I had something like this for the options:

• United Nations official languages
• Predefined languages (107 languages)
• Enabled languages (2 languages

and the description text:

"Languages to show in the language dropdown. United Nations official languages are the six official languages of the UN. Predefined languages are the @count_predefined predefined languages in Drupal. Enabled languages are the @count_enabled languages currently configured in Drupal, including site custom languages."

I didn't want to mess up any work on the existing MR, so I've opened a fresh MR against 11.x with a quick rebase. Sharing mostly to save someone else having to repeat dealing with the conflicts that popped up for me: !4656.

The rebased-3273986-pp-1-third-option-10.1.x branch can be ignored or deleted.

I hope to take a closer look at this during the week.

Thanks for pushing this forward again, @xurizaemon! Looking forward to reviewing this when you think it's ready for that 😊

I will have a shot at that @ifrik!

One thing I'd like to propose is rewording the options. I feel strongly that "all" is not an appropriate term here (as noted in #59 above), so in my MR I intend to provide one of the following proposed list of options:

I see that at /admin/config/regional/language Drupal uses "configured languages" not "enabled languages" so I will also rename the setting option from enabled to configured.

I plan to address this language change, only showing the third option when appropriate (site has languages configured), and test coverage.

My intent is to reduce unclear language by replacing "all" in user-exposed text, and to standardise the output (description, count).

I don't intend changing the use of all in the settings, as this would require additional changes (hook_update_N, schema) which feels unnecessary for a string which is not user-exposed.

Input welcome! If you prefer A or B or some third option, please say so here.

Changing title and ID to reflect proposed "configured" value, not "enabled". This more accurately reflects the language used at /admin/config/regional/language

Have added some tidy up and refactor to !4656 - trying to be a good scout here, happy of course to back out any changes which go beyond what is appropriate. The existing tests had one very long method which did a lot, and it was hard to follow the location of the browser moving through the test. Hoping the changes will make this clearer.

Earlier I'd said "Only show the third option when the site has configured languages"

But ... all sites have a minimum of one language, so this doesn't happen. What can happen is that the site doesn't have Language module enabled, but the site still shows "Site configured languages (1 language)", and I feel that hiding the option in this case would be less discoverable, so I left it as-is.

If Language module is enabled, then the settings screen description now links to the route which lets admins configure languages.

Considering future approaches such as the one mentioned in #3375157: [upstream] CKEditor 5 language selector cannot be scrolled if "All NNN languages" option selected I think "configured" would not be a clear label for the option we're introducing here. I think this label should be much clearer, eg "site_configured". ($languageManagerInterface->getLanguages() calls this "a list of languages set up on the site", so we have "enabled", "set up", "configured" but the important detail seems to be that this is the site configuration, where the idea in #3375157 might be an input format configuration.

(I'm not touching #3375157 here except to avoid using a label which would make introducing that less clear. Nor am I touching on "all" as it's pre-existing. I already thought that "all" was a poor label for the existing configuration value which refers to Drupal's standard languages. Perhaps "standard" might be better there, but I think that change is out of scope for this issue.)

I would appreciate input on improvements on "site_configured" (because I might be missing some nuance around that label).

Adding some screenshots for review discussion.

Administration > Configuration > Content authoring > Text formats and editors > {format} > CKEditor 5 Plugin settings > Language:

NB: If the language configuration route is not available (most likely because Language module is not enabled) then there will be 1 default language configured on the site, and the "configured on this site" text will not be linked.

When editing content:

For a site with 2 languages configured and "site_configured" option selected for the current text format, we see those languages as options:

Have an Umami setup currently running, which has 2 languages installed
Applied MR 4656
Verified I see the 3rd option in the language settings for the text format.
Created a node and verified the language dropdown shows just 2 languages.

LGTM!

Looks awesome!

I'm not a native speaker, but shouldn't it be Site-configured languages?

Needs work anyway for complying with one upcoming coding standard change, which if it'd land first would cause this MR to fail tests.

Please review.

Thanks!

I'm having trouble testing this due to #3388978: Improve warning upon adding the Language plugin and #3388698: Adding the language plug-in does not add the correct span tag attributes. Is there some other prerequisite for this? I don't want to derail this patch given that the two issues listed occur for me with or without the patch but is there something I'm missing?

I've related #3388698: Adding the language plug-in does not add the correct span tag attributes because the issue documented there occurs if I test the current issue. However, it is not a child of this issue because the issue documented there also occurs if I choose 107 languages and UN languages. However, I have a workaround, documented in that issue, which works if I default to the UN languages but not if I choose 107 languages. But the workaround also doesn't work if I install the patch for the current issue and choose site-configured languages.

Steps for that issue when testing with the patch in the current issue:

1. Go to https://simplytest.me/
2. In the field Evaluate Drupal Projects, type Drupal
3. Choose Drupal core
4. Accept 10.1.3 (or whatever the latest version offered is) as the default
5. Click Advanced options
6. In the field Add patches on the chosen project, paste https://git.drupalcode.org/project/drupal/-/merge_requests/4656.patch
7. Click Launch sandbox
8. Wait to be redirected to the test instance
9. Enter the admin user name and password
10. Click Extend
11. Under Multilingual, check Content Translation and Language
12. Click Install
13. Wait until the process is done
14. Click Configuration, then Languages
15. Click Add language
16. In the Language name drop-down, choose Custom Language
17. Type tl (that is, lower-case L) for the Language Code and Filipino for the Language Name.
18. Click Add Custom Language
19. Click Configuration, then Text formats and editors
20. On the row for basic HTML, click Configure
21. Drag the Language button into the active toolbar

Result: Warning message
The Language plugin needs another plugin to create , for it to be able to create the following attributes: . Enable a plugin that supports creating this tag. If none exists, you can configure the Source Editing plugin to support it.

22. Under CKEditor5 plugin settings, click Language
23. In the drop-down, choose Site-configured languages (2)
24. Click Save Configuration

Result: Error message
The Language plugin needs another plugin to create , for it to be able to create the following attributes: . Enable a plugin that supports creating this tag. If none exists, you can configure the Source Editing plugin to support it.

Attempted workaround:

25. Under CKEditor5 plugin settings, click Source editing
26. Change

<cite> <dl> <dt> <dd> <a hreflang> <blockquote cite> <ul type> <ol start type> <h2 id> <h3 id> <h4 id> <h5 id> <h6 id>

to read

<cite> <dl> <dt> <dd> <a hreflang> <blockquote cite> <ul type> <ol start type> <h2 id> <h3 id> <h4 id> <h5 id> <h6 id> <span lang dir>

27. Click Save Configuration

Expected results:

• Error message disappears.
• Source editing setting continues to read:
  <cite> <dl> <dt> <dd> <a hreflang> <blockquote cite> <ul type> <ol start type> <h2 id> <h3 id> <h4 id> <h5 id> <h6 id> <span lang dir>

Actual results:

• Error message remains.
• Source editing setting reads:
  <cite> <dl> <dt> <dd> <a hreflang> <blockquote cite> <ul type> <ol start type> <h2 id> <h3 id> <h4 id> <h5 id> <h6 id>

#81: Now that you mention it, I recall hitting that behaviour as well! However I believe I ran into it before applying the changes in this issue, when initially setting up the Language of Parts plugin. So #3388698: Adding the language plug-in does not add the correct span tag attributes for me isn't a blocker or issue here.

#81: that's unrelated to this issue. Just responded at #3388978-5: Improve warning upon adding the Language plugin — let's address that usability problem there, let's not derail this issue 🙏 (with your admittedly wonderfully detailed steps to reproduce! 👍)

Wim Leers kindly pointed my misunderstanding of how adding plugins is supposed to work on my issue #3388978: Improve warning upon adding the Language plugin. I corrected step 26 in my preceding comment on the current issue from <span lang dir> to <span> and the steps worked!

Adding one more vote to reviewed and tested by the community.

🥳 Now let's get that UI string improved in #3388978: Improve warning upon adding the Language plugin! 🤝

Asked question on the MR

Moving back to RTBC per my response on MR. @laurii, I don't think that should prevent this useful fix landing.

Thanks @xurizaemon! Makes sense 👍

Committed e1fc76a and pushed to 11.x. Thanks!

Thanks! Happy to see this in :)

Hi,

I see that this is fixed but it's been released on the 11.x-dev branch. Could someone make available a patch for Drupal 10 in the meantime?

Thank you

@tariqDev I suggest to try the patch available from https://git.drupalcode.org/project/drupal/-/commit/e1fc76a1d2d9ff925bdbb... which may apply fine to your 10.x site.

I obtain the URL from the commit link a couple comments above yours + appending a .diff suffix to the URL.