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.