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

Rerolling during Global Contribution Weekend today

ifrik → closed merge request !2689

ifrik → closed merge request !3325

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? 😊🙏

There are some coding standard errors in the branch that I pushed yesterday and I'll fix those myself.

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

Thanks, I will do that.

Obviously it fails if I only change half of the test..

Still one last test failure 😅

And … AFAICT the feedback has not yet been addressed?

I'm working on this.

Hopefully that fixed the issue of the text direction.
The changes for the functional test are still underway.

Marked 🐛 CKEditor5 language selector unusable Closed: duplicate and ✨ Add option to use website languages as the CKEditor5 drop-down languages Closed: duplicate 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 .

Wim Leers → credited Charles Belov → .

Wim Leers → credited jonathan_hunt → .

Updating title from CKEditor 4 to CKEditor 5.

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 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 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)

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

In 🐛 Add option to select from Drupal's enabled languages to CKEditor "Language" button Closed: duplicate 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.

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 😊

Thanks, I got stuck on writing tests.

I will have a shot at that @ifrik!

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

Input welcome! My intent is to remove unclear language in using "all", and to standardise the output.

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

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 my MR - 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.

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.

Considering future approaches such as the one mentioned in 🐛 CKEditor5 language selector unusable Closed: duplicate I think "configured" would be a similarly poor label for the one 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 would appreciate input on improvements on "site_configured" (because I might be missing some nuance around that label).

Screenshots -

Options as displayed on text format settings CKEditor > Language tab:

Description as shown on text format CKEditor Language tab:

Language selector with site's configured 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 ?

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 🐛 Eliminate or improve error upon adding the language plug-in Active and 🐛 Following instructions for error upon adding the language plug-in does not remedy the issue Postponed: needs info . 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 🐛 Following instructions for error upon adding the language plug-in does not remedy the issue Postponed: needs info 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 🐛 Following instructions for error upon adding the language plug-in does not remedy the issue Postponed: needs info for me isn't a blocker or issue here.

#81: that's unrelated to this issue. Just responded at #3388978-5: Eliminate or improve error upon adding the language plug-in → — 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 🐛 Eliminate or improve error upon adding the language plug-in Active on my issue 🐛 Eliminate or improve error upon adding the language plug-in Active . I corrected step 26 in my preceding comment on the current issue ✨ Third option for the CKEditor 5 "Language" button: `site_configured` (in addition to `un` and `all`) RTBC 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 🐛 Eliminate or improve error upon adding the language plug-in Active ! 🤝

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.

lauriii → closed merge request !4656

Thanks @xurizaemon! Makes sense 👍

Committed e1fc76a and pushed to 11.x. Thanks!

Thanks! Happy to see this in :)

Automatically closed - issue fixed for 2 weeks with no activity.

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.