Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

editor builder defaultConfig.language not apply to editor.locale #8510

Closed
xinglie opened this issue Nov 23, 2020 · 5 comments · Fixed by #10799
Closed

editor builder defaultConfig.language not apply to editor.locale #8510

xinglie opened this issue Nov 23, 2020 · 5 comments · Fixed by #10799
Assignees
Labels
squad:platform Issue to be handled by the Platform team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@xinglie
Copy link

xinglie commented Nov 23, 2020

📝 Provide detailed reproduction steps (if any)

  1. use ckeditor5/packages/ckeditor5-build-decoupled-document to build custom editor
  2. change ckeditor5/packages/ckeditor5-build-decoupled-document/src/ckeditor.js the defaultConfig.language to zh-cn

✔️ Expected result

ui is zh-cn and editor.locale also too

❌ Actual result

only ui display language in zh-cn . editor.locale language still is en,both uiLanguage and cotentLanguage

📃 Other details


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@xinglie xinglie added the type:bug This issue reports a buggy (incorrect) behavior. label Nov 23, 2020
@FilipTokarski
Copy link
Member

Hi, thanks for the report. Indeed after changing the language the editor.locale still shows en.
cc @ma2ciek could you take a look at this?

@ma2ciek ma2ciek self-assigned this Nov 26, 2020
@ma2ciek ma2ciek added the squad:platform Issue to be handled by the Platform team. label Nov 27, 2020
@mlewand mlewand added this to the nice-to-have milestone Dec 7, 2020
@Reinmar
Copy link
Member

Reinmar commented Oct 5, 2021

I confirm that it doesn't work. And it's odd.

Only setting it in create() works.

@pomek
Copy link
Member

pomek commented Oct 5, 2021

📝 Steps

  • Use the ckeditor5-build-classic, but it happens with other builds too.

  • Start the HTTP server in the packages/ckeditor5-build-classic directory (hs .)

  • In the sample/index.html file, modify script tags:

    <script src="../build/ckeditor.js"></script>
    <script src="../build/translations/pl.js"></script>
    <script>
        ClassicEditor.defaultConfig.language = 'pl';
        ClassicEditor.create( document.querySelector( '#editor' ) )
      	  .then( editor => {
      		  window.editor = editor;
      	  } )
      	  .catch( error => {
      		  console.error( 'There was a problem initializing the editor.', error );
      	  } );
    </script>
    
  • Type editor.locale in the console.

✔️ Expected result

  editor.locale
  Locale {uiLanguage: 'pl', contentLanguage: 'pl', uiLanguageDirection: 'ltr', contentLanguageDirection: 'ltr', t: ƒ}

❌ Actual result

  editor.locale
  Locale {uiLanguage: 'en', contentLanguage: 'en', uiLanguageDirection: 'ltr', contentLanguageDirection: 'ltr', t: ƒ}

❓ Possible solution

  • Find a place where the editor.locale object is created.
  • Notice that Editor#defaultConfig is used too late, and the Context class is already initialized.
  • The goal is to use the language specified in the configuration object passed in the constructor or built in the editor (Editor#defaultConfig).
Solution (SPOILER)
diff --git a/packages/ckeditor5-core/src/editor/editor.js b/packages/ckeditor5-core/src/editor/editor.js
index 141a7076f2..c4677263c4 100644
--- a/packages/ckeditor5-core/src/editor/editor.js
+++ b/packages/ckeditor5-core/src/editor/editor.js
@@ -52,6 +52,8 @@ export default class Editor {
 	 * @param {Object} [config={}] The editor configuration.
 	 */
 	constructor( config = {} ) {
+		const language = config.language || this.constructor.defaultConfig.language;
+
 		/**
 		 * The editor context.
 		 * When it is not provided through the configuration, the editor creates it.
@@ -59,7 +61,7 @@ export default class Editor {
 		 * @protected
 		 * @type {module:core/context~Context}
 		 */
-		this._context = config.context || new Context( { language: config.language } );
+		this._context = config.context || new Context( { language } );
 		this._context._addEditor( this, !config.context );
 
 		// Clone the plugins to make sure that the plugin array will not be shared

Make sure to cover this change with tests:

  • passing the language in the constructor
  • specifying the language as #defaultConfig
  • specifying the language by passing an instance of the Context class in the constructor as config.context

@pomek
Copy link
Member

pomek commented Oct 5, 2021

☝️ remember that the build is not refreshed when applying changes in its sources.

Please call the yarn run build --mode development --watch command in the build directory to automatically rebuild the build when changing the code.

@pomek pomek modified the milestones: nice-to-have, iteration 48, backlog, iteration 49 Oct 22, 2021
@psmyrek psmyrek self-assigned this Nov 3, 2021
przemyslaw-zan added a commit that referenced this issue Nov 5, 2021
Fix (core): Support language configuration passed in defaultConfig option through editor's constructor. Closes #8510.
@wimleers
Copy link

This might solve https://www.drupal.org/project/drupal/issues/3250191 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad:platform Issue to be handled by the Platform team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
8 participants