Opened 11 years ago
Closed 11 years ago
#11911 closed Bug (fixed)
[PR#98] Setting of the "dir" attribute in the preloaded lang JS case
| Reported by: | Marek Lewandowski | Owned by: | Marek Lewandowski |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 4.4.1 |
| Component: | UI : Language | Version: | |
| Keywords: | Cc: |
Description
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.
- open AJAX sample with CKEditor (i.e. samples/ajax.html)
- execute following code in console:
CKEDITOR.lang[ CKEDITOR.lang.detect() ] = { common: {} } - Click "Create Editor" button to init first CKE instance
- 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.
Change History (6)
comment:1 Changed 11 years ago by
| Owner: | set to Marek Lewandowski |
|---|---|
| Status: | new → assigned |
comment:2 Changed 11 years ago by
| Status: | assigned → review |
|---|
comment:3 Changed 11 years ago by
| Milestone: | → CKEditor 4.4.1 |
|---|---|
| Status: | review → review_failed |
- There are missing spaces in closure in tests.
- I don't think we need to mock
lang.detect- I commented that part out and it worked. - We should write assertion messages as positive statements - what should be the desirable state.
The dev changes are ok.
comment:4 Changed 11 years ago by
| Status: | review_failed → review |
|---|
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.
comment:5 Changed 11 years ago by
| Status: | review → review_passed |
|---|
Fix the message:
+ assert.areEqual( 'ltr', CKEDITOR.lang.en.dir, 'lang.ar.dir' );
And remember to close the PR.
comment:6 Changed 11 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Fixed with git:5b1b8ed65b (merged to master) at dev and 246c4aaef0 (merged to master) at tests.

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.