#8163 closed Bug (fixed)
filebrowserWindowFeatures
| Reported by: | Michal Malének | Owned by: | Sa'ar Zac Elias |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 3.6.2 |
| Component: | General | Version: | 3.4.1 |
| Keywords: | Cc: |
Description
I've noticed that there is no 'filebrowserWindowFeatures' - instead there is fileBrowserWindowFeatures (capital B) while all other "filebrowser" parameters are spelled in lower case. Is this intentional? (it took me pretty long before I realized that fileBrowserWindowFeatures was misspelled.)
Attachments (2)
Change History (10)
comment:1 Changed 13 years ago by
| Status: | new → confirmed |
|---|---|
| Version: | → 3.4.1 |
Changed 13 years ago by
| Attachment: | 8163.patch added |
|---|
comment:2 Changed 13 years ago by
| Owner: | set to Sa'ar Zac Elias |
|---|---|
| Status: | confirmed → review |
comment:3 Changed 13 years ago by
| Milestone: | → CKEditor 3.6.2 |
|---|
comment:4 Changed 13 years ago by
| Owner: | changed from Sa'ar Zac Elias to Anna Tomanek |
|---|---|
| Status: | review → assigned |
After yet another discussion with Wiktor we have decided to skip the backwards compatibility code as it does not seem that anyone used the incorrect version given that in the documentation it was spelled differently. It does not seem to make much sense to introduce redundant code then.
The new patch will correct the spelling and the API documentation.
Changed 13 years ago by
| Attachment: | 8163_2.patch added |
|---|
comment:5 Changed 13 years ago by
| Owner: | changed from Anna Tomanek to Sa'ar Zac Elias |
|---|---|
| Status: | assigned → review |
We have no way of knowing no one uses that configuration, and we cannot just "assume" so, given the long run this configuration exists. It's very important to keep the API signature as-is. Wiktor and I agreed, then, that for now back-compat should be preserved, and removed during the v4 development.
Doc changes are to be done regardless the ticket.
comment:6 Changed 13 years ago by
| Status: | review → review_passed |
|---|
If so, I think we are good to go. A minor note, though, can we change the comment to include the full and proper version of the "backward compatibility" keyword to make it easier to find later?
// TODO: V4: Remove backward compatibility (#8163).
comment:7 Changed 13 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Fixed with [7173].
This definitely is not intentional and what is more, API docs are even more misleading by giving the name of this configuration setting as filebrowserWindowFeatures.
As discussed with Wiktor, the code should be corrected in a way that would preserve backwards-compatibility with existing customization code that could be using the wrong spelling.