[PR#98] Setting of the "dir" attribute in the preloaded lang JS case

[Marek Lewandowski]

GitHub user akashmohapatra created a pull request for following issue:

If language dictionary is predefined in CKEDITOR.lang object, it will not set CKEDITOR.lang[ '<langId>' ].dir property.

1. open AJAX sample with CKEditor (i.e. samples/ajax.html)
2. execute following code in console:

   CKEDITOR.lang[ CKEDITOR.lang.detect() ] = { common: {} }
   

3. Click "Create Editor" button to init first CKE instance
4. execute following code in console

   CKEDITOR.lang[ CKEDITOR.lang.detect() ].dir
   

Expected result:
This statement should return a string: "ltr" (or "rtl" in case of using rtl lang)

Current result:
Variable is not assigned: undefined

Pull request can be found at ​GitHub.

[Marek Lewandowski]

I added few minor fixes to proposed solution, and removed unused variable in CKEDITOR.lang.

Pushed to ​t/11911 at dev and t/11911 at tests.

[Piotrek Koszuliński]

1. There are missing spaces in closure in tests.
2. I don't think we need to mock lang.detect - I commented that part out and it worked.
3. We should write assertion messages as positive statements - what should be the desirable state.

The dev changes are ok, but you could squash the commit fixing code style into previous one.

[Marek Lewandowski]

Yea that's true, mocking was not needed in this case. I've removed it, fixed codestyle and squashed commits.

Rebased and force pushed both branches.

[Piotrek Koszuliński]

Fix the message:

+                        assert.areEqual( 'ltr', CKEDITOR.lang.en.dir, 'lang.ar.dir' );

And remember to close the PR.

[Marek Lewandowski]

Fixed with ​git:5b1b8ed65b (merged to master) at dev and 246c4aaef0 (merged to master) at tests.